| Message ID | 20231127125726.3735-18-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net/lwip: add lwip library for the network stack | expand |
On Mon, Nov 27, 2023 at 06:57:00PM +0600, Maxim Uvarov wrote: > Add additional checks for NULL pointers. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > drivers/net/sandbox.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > index 13022addb6..75d32db3a9 100644 > --- a/drivers/net/sandbox.c > +++ b/drivers/net/sandbox.c > @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > struct ethernet_hdr *eth_recv; > struct arp_hdr *arp_recv; > > + if (!priv) > + return -EAGAIN; > + > if (ntohs(eth->et_protlen) != PROT_ARP) > return -EAGAIN; This part seems fine. > @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > > /* Formulate a fake response */ > eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; > + if (!eth_recv) > + return -EAGAIN; > memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); > memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); > eth_recv->et_protlen = htons(PROT_ARP); How do we get to this dereference, and is that not a bug in the caller?
Hi Maxim, On Mon, 27 Nov 2023 at 11:20, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Nov 27, 2023 at 06:57:00PM +0600, Maxim Uvarov wrote: > > Add additional checks for NULL pointers. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > drivers/net/sandbox.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > index 13022addb6..75d32db3a9 100644 > > --- a/drivers/net/sandbox.c > > +++ b/drivers/net/sandbox.c > > @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > > struct ethernet_hdr *eth_recv; > > struct arp_hdr *arp_recv; > > > > + if (!priv) > > + return -EAGAIN; > > + > > if (ntohs(eth->et_protlen) != PROT_ARP) > > return -EAGAIN; > > This part seems fine. > > > @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > > > > /* Formulate a fake response */ > > eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; > > + if (!eth_recv) > > + return -EAGAIN; > > memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); > > memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); > > eth_recv->et_protlen = htons(PROT_ARP); > > How do we get to this dereference, and is that not a bug in the caller? I wonder if somehow the device has not been probed yet? Regards, Simon
On Sat, Dec 02, 2023 at 11:33:14AM -0700, Simon Glass wrote: > Hi Maxim, > > On Mon, 27 Nov 2023 at 11:20, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Nov 27, 2023 at 06:57:00PM +0600, Maxim Uvarov wrote: > > > Add additional checks for NULL pointers. > > > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > > --- > > > drivers/net/sandbox.c | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > > index 13022addb6..75d32db3a9 100644 > > > --- a/drivers/net/sandbox.c > > > +++ b/drivers/net/sandbox.c > > > @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > > > struct ethernet_hdr *eth_recv; > > > struct arp_hdr *arp_recv; > > > > > > + if (!priv) > > > + return -EAGAIN; > > > + > > > if (ntohs(eth->et_protlen) != PROT_ARP) > > > return -EAGAIN; > > > > This part seems fine. > > > > > @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, > > > > > > /* Formulate a fake response */ > > > eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; > > > + if (!eth_recv) > > > + return -EAGAIN; > > > memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); > > > memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); > > > eth_recv->et_protlen = htons(PROT_ARP); > > > > How do we get to this dereference, and is that not a bug in the caller? > > I wonder if somehow the device has not been probed yet? Given the failures on a number of real hardware platforms in v11 as well (which I didn't see until after my review), I wonder if you've not spotted the cause of all of those other failures?
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index 13022addb6..75d32db3a9 100644 --- a/drivers/net/sandbox.c +++ b/drivers/net/sandbox.c @@ -65,6 +65,9 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, struct ethernet_hdr *eth_recv; struct arp_hdr *arp_recv; + if (!priv) + return -EAGAIN; + if (ntohs(eth->et_protlen) != PROT_ARP) return -EAGAIN; @@ -82,6 +85,8 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet, /* Formulate a fake response */ eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets]; + if (!eth_recv) + return -EAGAIN; memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN); memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN); eth_recv->et_protlen = htons(PROT_ARP);
Add additional checks for NULL pointers. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- drivers/net/sandbox.c | 5 +++++ 1 file changed, 5 insertions(+)