| Message ID | 20231225153929.8284-3-maxim.uvarov@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | net fixes prior lwip | expand |
On 12/25/23 10:39, Maxim Uvarov wrote: > Add additional checks for NULL pointers. > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > --- > drivers/net/sandbox.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > index 13022addb6..d91935e032 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; > + When can priv be NULL? --Sean > if (ntohs(eth->et_protlen) != PROT_ARP) > return -EAGAIN; >
On Tue, 26 Dec 2023 at 04:43, Sean Anderson <seanga2@gmail.com> wrote: > On 12/25/23 10:39, Maxim Uvarov wrote: > > Add additional checks for NULL pointers. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> > > --- > > drivers/net/sandbox.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > index 13022addb6..d91935e032 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; > > + > > When can priv be NULL? > > --Sean > > Function struct eth_sandbox_priv *priv = dev_get_priv(dev) can return NULL. If you ask why it doesn't return NULL without lwip patches and can return NULL with lwip patch while there is no clear code dependency.. Then I can not say right now and need additional investigation. But anyway the return code of dev_dev_priv() has to be checked I think. BR, Maxim. > > if (ntohs(eth->et_protlen) != PROT_ARP) > > return -EAGAIN; > > > >
On 12/26/23 01:18, Maxim Uvarov wrote: > > > On Tue, 26 Dec 2023 at 04:43, Sean Anderson <seanga2@gmail.com <mailto:seanga2@gmail.com>> wrote: > > On 12/25/23 10:39, Maxim Uvarov wrote: > > Add additional checks for NULL pointers. > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org <mailto:maxim.uvarov@linaro.org>> > > --- > > drivers/net/sandbox.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > index 13022addb6..d91935e032 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; > > + > > When can priv be NULL? > > --Sean > > > Function > struct eth_sandbox_priv *priv = dev_get_priv(dev) > can return NULL. If you ask why it doesn't return NULL without lwip patches and can return NULL with lwip patch while there is no clear code dependency.. > Then I can not say right now and need additional investigation. But anyway the return code of dev_dev_priv() has to be checked I think. If you set priv_auto to a nonzero value, dev_get_priv will always return non-null and does not need to be checked. So this is a NACK from me until you can justify this. --Sean
On Tue, 26 Dec 2023 at 12:25, Sean Anderson <seanga2@gmail.com> wrote: > On 12/26/23 01:18, Maxim Uvarov wrote: > > > > > > On Tue, 26 Dec 2023 at 04:43, Sean Anderson <seanga2@gmail.com <mailto: > seanga2@gmail.com>> wrote: > > > > On 12/25/23 10:39, Maxim Uvarov wrote: > > > Add additional checks for NULL pointers. > > > > > > Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org <mailto: > maxim.uvarov@linaro.org>> > > > --- > > > drivers/net/sandbox.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c > > > index 13022addb6..d91935e032 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; > > > + > > > > When can priv be NULL? > > > > --Sean > > > > > > Function > > struct eth_sandbox_priv *priv = dev_get_priv(dev) > > can return NULL. If you ask why it doesn't return NULL without lwip > patches and can return NULL with lwip patch while there is no clear code > dependency.. > > Then I can not say right now and need additional investigation. But > anyway the return code of dev_dev_priv() has to be checked I think. > > If you set priv_auto to a nonzero value, dev_get_priv will always return > non-null > and does not need to be checked. So this is a NACK from me until you can > justify this. > > --Sean > Ok. I will remove this patch from v3 patchset and send it separately with a more detailed description. BR, Maxim.
diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c index 13022addb6..d91935e032 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;
Add additional checks for NULL pointers. Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org> --- drivers/net/sandbox.c | 3 +++ 1 file changed, 3 insertions(+)