Skip to content

Commit cbe860b

Browse files
DanMax03Paolo Abeni
authored andcommitted
net: atlantic: Fix NULL dereference of skb pointer in
If is_ptp_ring == true in the loop of __aq_ring_xdp_clean function, then a timestamp is stored from a packet in a field of skb object, which is not allocated at the moment of the call (skb == NULL). Generalize aq_ptp_extract_ts and other affected functions so they don't work with struct sk_buff*, but with struct skb_shared_hwtstamps*. Found by Linux Verification Center (linuxtesting.org) with SVACE Fixes: 26efaef ("net: atlantic: Implement xdp data plane") Signed-off-by: Daniil Maximov <[email protected]> Reviewed-by: Igor Russkikh <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Paolo Abeni <[email protected]>
1 parent 7037d95 commit cbe860b

File tree

3 files changed

+19
-13
lines changed

3 files changed

+19
-13
lines changed

drivers/net/ethernet/aquantia/atlantic/aq_ptp.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -553,17 +553,17 @@ void aq_ptp_tx_hwtstamp(struct aq_nic_s *aq_nic, u64 timestamp)
553553

554554
/* aq_ptp_rx_hwtstamp - utility function which checks for RX time stamp
555555
* @adapter: pointer to adapter struct
556-
* @skb: particular skb to send timestamp with
556+
* @shhwtstamps: particular skb_shared_hwtstamps to save timestamp
557557
*
558558
* if the timestamp is valid, we convert it into the timecounter ns
559559
* value, then store that result into the hwtstamps structure which
560560
* is passed up the network stack
561561
*/
562-
static void aq_ptp_rx_hwtstamp(struct aq_ptp_s *aq_ptp, struct sk_buff *skb,
562+
static void aq_ptp_rx_hwtstamp(struct aq_ptp_s *aq_ptp, struct skb_shared_hwtstamps *shhwtstamps,
563563
u64 timestamp)
564564
{
565565
timestamp -= atomic_read(&aq_ptp->offset_ingress);
566-
aq_ptp_convert_to_hwtstamp(aq_ptp, skb_hwtstamps(skb), timestamp);
566+
aq_ptp_convert_to_hwtstamp(aq_ptp, shhwtstamps, timestamp);
567567
}
568568

569569
void aq_ptp_hwtstamp_config_get(struct aq_ptp_s *aq_ptp,
@@ -639,7 +639,7 @@ bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring)
639639
&aq_ptp->ptp_rx == ring || &aq_ptp->hwts_rx == ring;
640640
}
641641

642-
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p,
642+
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
643643
unsigned int len)
644644
{
645645
struct aq_ptp_s *aq_ptp = aq_nic->aq_ptp;
@@ -648,7 +648,7 @@ u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p,
648648
p, len, &timestamp);
649649

650650
if (ret > 0)
651-
aq_ptp_rx_hwtstamp(aq_ptp, skb, timestamp);
651+
aq_ptp_rx_hwtstamp(aq_ptp, shhwtstamps, timestamp);
652652

653653
return ret;
654654
}

drivers/net/ethernet/aquantia/atlantic/aq_ptp.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ int aq_ptp_hwtstamp_config_set(struct aq_ptp_s *aq_ptp,
6767
/* Return either ring is belong to PTP or not*/
6868
bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring);
6969

70-
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p,
70+
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
7171
unsigned int len);
7272

7373
struct ptp_clock *aq_ptp_get_ptp_clock(struct aq_ptp_s *aq_ptp);
@@ -143,7 +143,7 @@ static inline bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring)
143143
}
144144

145145
static inline u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic,
146-
struct sk_buff *skb, u8 *p,
146+
struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
147147
unsigned int len)
148148
{
149149
return 0;

drivers/net/ethernet/aquantia/atlantic/aq_ring.c

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ static int __aq_ring_rx_clean(struct aq_ring_s *self, struct napi_struct *napi,
647647
}
648648
if (is_ptp_ring)
649649
buff->len -=
650-
aq_ptp_extract_ts(self->aq_nic, skb,
650+
aq_ptp_extract_ts(self->aq_nic, skb_hwtstamps(skb),
651651
aq_buf_vaddr(&buff->rxdata),
652652
buff->len);
653653

@@ -742,6 +742,8 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
742742
struct aq_ring_buff_s *buff = &rx_ring->buff_ring[rx_ring->sw_head];
743743
bool is_ptp_ring = aq_ptp_ring(rx_ring->aq_nic, rx_ring);
744744
struct aq_ring_buff_s *buff_ = NULL;
745+
u16 ptp_hwtstamp_len = 0;
746+
struct skb_shared_hwtstamps shhwtstamps;
745747
struct sk_buff *skb = NULL;
746748
unsigned int next_ = 0U;
747749
struct xdp_buff xdp;
@@ -810,11 +812,12 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
810812
hard_start = page_address(buff->rxdata.page) +
811813
buff->rxdata.pg_off - rx_ring->page_offset;
812814

813-
if (is_ptp_ring)
814-
buff->len -=
815-
aq_ptp_extract_ts(rx_ring->aq_nic, skb,
816-
aq_buf_vaddr(&buff->rxdata),
817-
buff->len);
815+
if (is_ptp_ring) {
816+
ptp_hwtstamp_len = aq_ptp_extract_ts(rx_ring->aq_nic, &shhwtstamps,
817+
aq_buf_vaddr(&buff->rxdata),
818+
buff->len);
819+
buff->len -= ptp_hwtstamp_len;
820+
}
818821

819822
xdp_init_buff(&xdp, frame_sz, &rx_ring->xdp_rxq);
820823
xdp_prepare_buff(&xdp, hard_start, rx_ring->page_offset,
@@ -834,6 +837,9 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
834837
if (IS_ERR(skb) || !skb)
835838
continue;
836839

840+
if (ptp_hwtstamp_len > 0)
841+
*skb_hwtstamps(skb) = shhwtstamps;
842+
837843
if (buff->is_vlan)
838844
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
839845
buff->vlan_rx_tag);

0 commit comments

Comments
 (0)