* Re: Re: [PATCH] net: fix null pointer dereference when parsing ICMP6_ROUTER_ADVERTISE messages
@ 2022-03-16 12:23 zhangqiumiao
2022-04-14 17:04 ` Daniel Kiper
0 siblings, 1 reply; 2+ messages in thread
From: zhangqiumiao @ 2022-03-16 12:23 UTC (permalink / raw)
To: Daniel Kiper, Glenn Washburn
Cc: Qiumiao Zhang via Grub-devel, Fengtao (fengtao, Euler),
Chenxiang (EulerOS), Yanan (Euler)
On Thu, Feb 17, 2022 at 03:32:52PM -0600, Glenn Washburn wrote:
> On Thu, 17 Feb 2022 21:48:58 +0800
> Qiumiao Zhang via Grub-devel <grub-devel@gnu.org> wrote:
>
> > During UEFI PXE boot in IPv6 network, if the DHCP server adopts
> > stateful automatic configuration, when the client receives the
> > ICMP6_ROUTER_ADVERTISE message multicast from the server, it will
> > cause the problem of dereference null pointer and cause the grub2 program to crash.
>
> This commit message could be more clear. Maybe have something like:
>
> During UEFI PXE boot in IPv6 network, if the DHCP server adopts
> stateful automatic configuration, then the client receives a
> ICMP6_ROUTER_ADVERTISE multicast message from the server. This may be
> received without the interfaced having a configured network address,
> so orig_inf will be null, which can lead to a null dereference when
> creating the default route.
>
Yes, that's a better description.
> Of course, assuming that the above is in fact correct.
>
> >
> > Fixes bug: https://savannah.gnu.org/bugs/index.php?62072
> > ---
> > grub-core/net/icmp6.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c index
> > 2cbd95d..264fc4a 100644
> > --- a/grub-core/net/icmp6.c
> > +++ b/grub-core/net/icmp6.c
> > @@ -477,7 +477,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
> > *nb,
> >
> > /* May not have gotten slaac info, find a global address on this
> > card. */
> > - if (route_inf == NULL)
> > + if (route_inf == NULL && orig_inf != NULL)
>
> So if orig_inf == NULL and route_inf == NULL here, we do not set a
> default route. Does this have any implications to be concerned about?
>
> In this case, can we still find a good route interface and setup a
> default route?
If the DHCP server adopts stateful automatic configuration, the client obtains the default route through DHCPv6 instead of RA messages. So if orig_inf == NULL and route_inf == null, we should not set the default route.
Qiumiao
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: Re: [PATCH] net: fix null pointer dereference when parsing ICMP6_ROUTER_ADVERTISE messages
2022-03-16 12:23 Re: [PATCH] net: fix null pointer dereference when parsing ICMP6_ROUTER_ADVERTISE messages zhangqiumiao
@ 2022-04-14 17:04 ` Daniel Kiper
0 siblings, 0 replies; 2+ messages in thread
From: Daniel Kiper @ 2022-04-14 17:04 UTC (permalink / raw)
To: zhangqiumiao1
Cc: grub-devel, Glenn Washburn, Fengtao (fengtao, Euler),
Chenxiang (EulerOS), Yanan (Euler)
On Wed, Mar 16, 2022 at 12:23:12PM +0000, zhangqiumiao via Grub-devel wrote:
> On Thu, Feb 17, 2022 at 03:32:52PM -0600, Glenn Washburn wrote:
> > On Thu, 17 Feb 2022 21:48:58 +0800
> > Qiumiao Zhang via Grub-devel <grub-devel@gnu.org> wrote:
> >
> > > During UEFI PXE boot in IPv6 network, if the DHCP server adopts
> > > stateful automatic configuration, when the client receives the
> > > ICMP6_ROUTER_ADVERTISE message multicast from the server, it will
> > > cause the problem of dereference null pointer and cause the grub2 program to crash.
> >
> > This commit message could be more clear. Maybe have something like:
> >
> > During UEFI PXE boot in IPv6 network, if the DHCP server adopts
> > stateful automatic configuration, then the client receives a
> > ICMP6_ROUTER_ADVERTISE multicast message from the server. This may be
> > received without the interfaced having a configured network address,
> > so orig_inf will be null, which can lead to a null dereference when
> > creating the default route.
>
> Yes, that's a better description.
>
> > Of course, assuming that the above is in fact correct.
> >
> > > Fixes bug: https://savannah.gnu.org/bugs/index.php?62072
> > > ---
> > > grub-core/net/icmp6.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/net/icmp6.c b/grub-core/net/icmp6.c index
> > > 2cbd95d..264fc4a 100644
> > > --- a/grub-core/net/icmp6.c
> > > +++ b/grub-core/net/icmp6.c
> > > @@ -477,7 +477,7 @@ grub_net_recv_icmp6_packet (struct grub_net_buff
> > > *nb,
> > >
> > > /* May not have gotten slaac info, find a global address on this
> > > card. */
> > > - if (route_inf == NULL)
> > > + if (route_inf == NULL && orig_inf != NULL)
> >
> > So if orig_inf == NULL and route_inf == NULL here, we do not set a
> > default route. Does this have any implications to be concerned about?
> >
> > In this case, can we still find a good route interface and setup a
> > default route?
>
> If the DHCP server adopts stateful automatic configuration, the client obtains the default route through DHCPv6 instead of RA messages. So if orig_inf == NULL and route_inf == null, we should not set the default route.
I am happy to take this fix but I am still waiting for updated patch
from you addressing all concerns voiced in the comments.
Daniel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-04-14 17:04 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-16 12:23 Re: [PATCH] net: fix null pointer dereference when parsing ICMP6_ROUTER_ADVERTISE messages zhangqiumiao
2022-04-14 17:04 ` Daniel Kiper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.