| Message ID | 2f2b4550-8c66-4300-85b5-b9143cc7d918@infradead.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v5,net] ps3/gelic: Fix SKB allocation | expand |
Le 19/02/2024 à 10:12, Geoff Levand a écrit : > Commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures") of > 6.8-rc1 had a copy-and-paste error where the pointer that holds the > allocated SKB (struct gelic_descr.skb) was set to NULL after the SKB was > allocated. This resulted in a kernel panic when the SKB pointer was > accessed. > > This fix moves the initialization of the gelic_descr to before the SKB > is allocated. > > Reported-by: sambat goson <sombat3960@gmail.com> > Fixes: 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures") > Signed-off-by: Geoff Levand <geoff@infradead.org> > > diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c > index d5b75af163d3..28116891d2ce 100644 > --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c > +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c > @@ -384,11 +384,6 @@ static int gelic_descr_prepare_rx(struct gelic_card *card, > if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) > dev_info(ctodev(card), "%s: ERROR status\n", __func__); > > - descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size); > - if (!descr->skb) { > - descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */ > - return -ENOMEM; > - } > descr->hw_regs.dmac_cmd_status = 0; > descr->hw_regs.result_size = 0; > descr->hw_regs.valid_size = 0; > @@ -397,6 +392,12 @@ static int gelic_descr_prepare_rx(struct gelic_card *card, > descr->hw_regs.payload.size = 0; > descr->skb = NULL; You are unconditionaly re-assigning value to descr->skb below, so above line is useless and should be removed. > > + descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size); > + if (!descr->skb) { > + descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */ > + return -ENOMEM; > + } > + Is this code move needed at all ? At the end, isn't it enough to just drop the line descr->skb = NULL; ? Christophe > offset = ((unsigned long)descr->skb->data) & > (GELIC_NET_RXBUF_ALIGN - 1); > if (offset)
diff --git a/drivers/net/ethernet/toshiba/ps3_gelic_net.c b/drivers/net/ethernet/toshiba/ps3_gelic_net.c index d5b75af163d3..28116891d2ce 100644 --- a/drivers/net/ethernet/toshiba/ps3_gelic_net.c +++ b/drivers/net/ethernet/toshiba/ps3_gelic_net.c @@ -384,11 +384,6 @@ static int gelic_descr_prepare_rx(struct gelic_card *card, if (gelic_descr_get_status(descr) != GELIC_DESCR_DMA_NOT_IN_USE) dev_info(ctodev(card), "%s: ERROR status\n", __func__); - descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size); - if (!descr->skb) { - descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */ - return -ENOMEM; - } descr->hw_regs.dmac_cmd_status = 0; descr->hw_regs.result_size = 0; descr->hw_regs.valid_size = 0; @@ -397,6 +392,12 @@ static int gelic_descr_prepare_rx(struct gelic_card *card, descr->hw_regs.payload.size = 0; descr->skb = NULL; + descr->skb = netdev_alloc_skb(*card->netdev, rx_skb_size); + if (!descr->skb) { + descr->hw_regs.payload.dev_addr = 0; /* tell DMAC don't touch memory */ + return -ENOMEM; + } + offset = ((unsigned long)descr->skb->data) & (GELIC_NET_RXBUF_ALIGN - 1); if (offset)
Commit 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures") of 6.8-rc1 had a copy-and-paste error where the pointer that holds the allocated SKB (struct gelic_descr.skb) was set to NULL after the SKB was allocated. This resulted in a kernel panic when the SKB pointer was accessed. This fix moves the initialization of the gelic_descr to before the SKB is allocated. Reported-by: sambat goson <sombat3960@gmail.com> Fixes: 3ce4f9c3fbb3 ("net/ps3_gelic_net: Add gelic_descr structures") Signed-off-by: Geoff Levand <geoff@infradead.org>