lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 20 Dec 2012 01:25:51 +0900 From: YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org> To: Bjørn Mork <bjorn@...k.no> CC: davem@...emloft.net, netdev@...r.kernel.org, YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org> Subject: Re: [GIT PULL net-next 04/17] ndisc: Introduce ndisc_fill_redirect_hdr_option(). Bjørn Mork wrote: > YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org> writes: > >> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@...ux-ipv6.org> >> --- >> net/ipv6/ndisc.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c >> index a181113..0a4f3a9 100644 >> --- a/net/ipv6/ndisc.c >> +++ b/net/ipv6/ndisc.c >> @@ -1332,6 +1332,19 @@ static void ndisc_redirect_rcv(struct sk_buff *skb) >> icmpv6_notify(skb, NDISC_REDIRECT, 0, 0); >> } >> >> +static u8 *ndisc_fill_redirect_hdr_option(u8 *opt, struct sk_buff *orig_skb, >> + int rd_len) >> +{ >> + memset(opt, 0, 8); >> + *(opt++) = ND_OPT_REDIRECT_HDR; >> + *(opt++) = (rd_len >> 3); >> + opt += 6; >> + >> + memcpy(opt, ipv6_hdr(orig_skb), rd_len - 8); >> + >> + return opt; >> +} >> + > > I realize that it doesn't currently matter, but the above modification > of "opt" looks like a bug-waiting-to-happen to me. > >> void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) >> { >> struct net_device *dev = skb->dev; >> @@ -1461,12 +1474,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target) >> * build redirect option and copy skb over to the new packet. >> */ >> >> - memset(opt, 0, 8); >> - *(opt++) = ND_OPT_REDIRECT_HDR; >> - *(opt++) = (rd_len >> 3); >> - opt += 6; >> - >> - memcpy(opt, ipv6_hdr(skb), rd_len - 8); >> + if (rd_len) >> + opt = ndisc_fill_redirect_hdr_option(opt, skb, rd_len); > > > I understand that opt isn't currently used after this, but if it ever is > then it is going to come as big a surprise that this implies opt += 8; > > This was previously quite clear when the code was inline, but it becomes > problematic when it is factored out. I understand your concern. opt will be disappeared by following changeset (12 of 17). --yoshfuji -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists