Commit cbe860be authored by Daniil Maximov's avatar Daniil Maximov Committed by Paolo Abeni

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: 26efaef7 ("net: atlantic: Implement xdp data plane")
Signed-off-by: default avatarDaniil Maximov <daniil31415it@gmail.com>
Reviewed-by: default avatarIgor Russkikh <irusskikh@marvell.com>
Link: https://lore.kernel.org/r/20231204085810.1681386-1-daniil31415it@gmail.comSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 7037d95a
...@@ -553,17 +553,17 @@ void aq_ptp_tx_hwtstamp(struct aq_nic_s *aq_nic, u64 timestamp) ...@@ -553,17 +553,17 @@ void aq_ptp_tx_hwtstamp(struct aq_nic_s *aq_nic, u64 timestamp)
/* aq_ptp_rx_hwtstamp - utility function which checks for RX time stamp /* aq_ptp_rx_hwtstamp - utility function which checks for RX time stamp
* @adapter: pointer to adapter struct * @adapter: pointer to adapter struct
* @skb: particular skb to send timestamp with * @shhwtstamps: particular skb_shared_hwtstamps to save timestamp
* *
* if the timestamp is valid, we convert it into the timecounter ns * if the timestamp is valid, we convert it into the timecounter ns
* value, then store that result into the hwtstamps structure which * value, then store that result into the hwtstamps structure which
* is passed up the network stack * is passed up the network stack
*/ */
static void aq_ptp_rx_hwtstamp(struct aq_ptp_s *aq_ptp, struct sk_buff *skb, static void aq_ptp_rx_hwtstamp(struct aq_ptp_s *aq_ptp, struct skb_shared_hwtstamps *shhwtstamps,
u64 timestamp) u64 timestamp)
{ {
timestamp -= atomic_read(&aq_ptp->offset_ingress); timestamp -= atomic_read(&aq_ptp->offset_ingress);
aq_ptp_convert_to_hwtstamp(aq_ptp, skb_hwtstamps(skb), timestamp); aq_ptp_convert_to_hwtstamp(aq_ptp, shhwtstamps, timestamp);
} }
void aq_ptp_hwtstamp_config_get(struct aq_ptp_s *aq_ptp, 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) ...@@ -639,7 +639,7 @@ bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring)
&aq_ptp->ptp_rx == ring || &aq_ptp->hwts_rx == ring; &aq_ptp->ptp_rx == ring || &aq_ptp->hwts_rx == ring;
} }
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p, u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
unsigned int len) unsigned int len)
{ {
struct aq_ptp_s *aq_ptp = aq_nic->aq_ptp; 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, ...@@ -648,7 +648,7 @@ u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p,
p, len, &timestamp); p, len, &timestamp);
if (ret > 0) if (ret > 0)
aq_ptp_rx_hwtstamp(aq_ptp, skb, timestamp); aq_ptp_rx_hwtstamp(aq_ptp, shhwtstamps, timestamp);
return ret; return ret;
} }
......
...@@ -67,7 +67,7 @@ int aq_ptp_hwtstamp_config_set(struct aq_ptp_s *aq_ptp, ...@@ -67,7 +67,7 @@ int aq_ptp_hwtstamp_config_set(struct aq_ptp_s *aq_ptp,
/* Return either ring is belong to PTP or not*/ /* Return either ring is belong to PTP or not*/
bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring); bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring);
u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct sk_buff *skb, u8 *p, u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
unsigned int len); unsigned int len);
struct ptp_clock *aq_ptp_get_ptp_clock(struct aq_ptp_s *aq_ptp); 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) ...@@ -143,7 +143,7 @@ static inline bool aq_ptp_ring(struct aq_nic_s *aq_nic, struct aq_ring_s *ring)
} }
static inline u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic, static inline u16 aq_ptp_extract_ts(struct aq_nic_s *aq_nic,
struct sk_buff *skb, u8 *p, struct skb_shared_hwtstamps *shhwtstamps, u8 *p,
unsigned int len) unsigned int len)
{ {
return 0; return 0;
......
...@@ -647,7 +647,7 @@ static int __aq_ring_rx_clean(struct aq_ring_s *self, struct napi_struct *napi, ...@@ -647,7 +647,7 @@ static int __aq_ring_rx_clean(struct aq_ring_s *self, struct napi_struct *napi,
} }
if (is_ptp_ring) if (is_ptp_ring)
buff->len -= buff->len -=
aq_ptp_extract_ts(self->aq_nic, skb, aq_ptp_extract_ts(self->aq_nic, skb_hwtstamps(skb),
aq_buf_vaddr(&buff->rxdata), aq_buf_vaddr(&buff->rxdata),
buff->len); buff->len);
...@@ -742,6 +742,8 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring, ...@@ -742,6 +742,8 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
struct aq_ring_buff_s *buff = &rx_ring->buff_ring[rx_ring->sw_head]; struct aq_ring_buff_s *buff = &rx_ring->buff_ring[rx_ring->sw_head];
bool is_ptp_ring = aq_ptp_ring(rx_ring->aq_nic, rx_ring); bool is_ptp_ring = aq_ptp_ring(rx_ring->aq_nic, rx_ring);
struct aq_ring_buff_s *buff_ = NULL; struct aq_ring_buff_s *buff_ = NULL;
u16 ptp_hwtstamp_len = 0;
struct skb_shared_hwtstamps shhwtstamps;
struct sk_buff *skb = NULL; struct sk_buff *skb = NULL;
unsigned int next_ = 0U; unsigned int next_ = 0U;
struct xdp_buff xdp; struct xdp_buff xdp;
...@@ -810,11 +812,12 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring, ...@@ -810,11 +812,12 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
hard_start = page_address(buff->rxdata.page) + hard_start = page_address(buff->rxdata.page) +
buff->rxdata.pg_off - rx_ring->page_offset; buff->rxdata.pg_off - rx_ring->page_offset;
if (is_ptp_ring) if (is_ptp_ring) {
buff->len -= ptp_hwtstamp_len = aq_ptp_extract_ts(rx_ring->aq_nic, &shhwtstamps,
aq_ptp_extract_ts(rx_ring->aq_nic, skb, aq_buf_vaddr(&buff->rxdata),
aq_buf_vaddr(&buff->rxdata), buff->len);
buff->len); buff->len -= ptp_hwtstamp_len;
}
xdp_init_buff(&xdp, frame_sz, &rx_ring->xdp_rxq); xdp_init_buff(&xdp, frame_sz, &rx_ring->xdp_rxq);
xdp_prepare_buff(&xdp, hard_start, rx_ring->page_offset, 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, ...@@ -834,6 +837,9 @@ static int __aq_ring_xdp_clean(struct aq_ring_s *rx_ring,
if (IS_ERR(skb) || !skb) if (IS_ERR(skb) || !skb)
continue; continue;
if (ptp_hwtstamp_len > 0)
*skb_hwtstamps(skb) = shhwtstamps;
if (buff->is_vlan) if (buff->is_vlan)
__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
buff->vlan_rx_tag); buff->vlan_rx_tag);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment