netdev - Re: [PATCH net-next] net: bridge: use mld2r_ngrec instead of icmpv6

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 29 Aug 2021 00:46:12 +0900
From:   진성희 <shjy180909@...il.com>
To:     Nikolay Aleksandrov <nikolay@...dia.com>
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        roopa@...dia.com
Subject: Re: [PATCH net-next] net: bridge: use mld2r_ngrec instead of icmpv6_dataun

2021년 8월 28일 (토) 오후 6:51, Nikolay Aleksandrov <nikolay@...dia.com>님이 작성:
>
> On 28/08/2021 11:43, shjy180909@...il.com wrote:
> > From: MichelleJin <shjy180909@...il.com>
> >
> > using icmp6h->mld2r_ngrec instead of icmp6h->icmp6_dataun.un_data16[1].
> >
> > Signed-off-by: MichelleJin <shjy180909@...il.com>
> > ---
> >  net/bridge/br_multicast.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> > index 2c437d4bf632..8e38e02208bd 100644
> > --- a/net/bridge/br_multicast.c
> > +++ b/net/bridge/br_multicast.c
> > @@ -2731,8 +2731,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
> >       struct net_bridge_mdb_entry *mdst;
> >       struct net_bridge_port_group *pg;
> >       unsigned int nsrcs_offset;
> > +     struct mld2_report *mld2r;
> >       const unsigned char *src;
> > -     struct icmp6hdr *icmp6h;
> >       struct in6_addr *h_addr;
> >       struct mld2_grec *grec;
> >       unsigned int grec_len;
> > @@ -2740,12 +2740,12 @@ static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
> >       int i, len, num;
> >       int err = 0;
> >
> > -     if (!ipv6_mc_may_pull(skb, sizeof(*icmp6h)))
> > +     if (!ipv6_mc_may_pull(skb, sizeof(*mld2r)))
> >               return -EINVAL;
> >
> > -     icmp6h = icmp6_hdr(skb);
> > -     num = ntohs(icmp6h->icmp6_dataun.un_data16[1]);
> > -     len = skb_transport_offset(skb) + sizeof(*icmp6h);
> > +     mld2r = (struct mld2_report *) icmp6_hdr(skb);
> > +     num = ntohs(mld2r->mld2r_ngrec);
> > +     len = skb_transport_offset(skb) + sizeof(*mld2r);
> >
> >       for (i = 0; i < num; i++) {
> >               __be16 *_nsrcs, __nsrcs;
> >
>
> Indeed it should be equivalent, have you run the bridge selftests with this change?
>
>

I did printk tests.

There's no difference between mld2r->mld2r_ngrec and

icmp6h->icmp6_dataun.un_data16[1].

Also, there is no difference between sizeof(*mld2r) and sizeof(*icmp6h).

test code:
static int br_ip6_multicast_mld2_report(struct net_bridge_mcast *brmctx,
                                        struct net_bridge_mcast_port *pmctx,
                                        struct sk_buff *skb,
                                        u16 vid)
{
...
        struct mld2_report *mld2r; //
        struct icmp6hdr *icmp6h; //
...

        mld2r = (struct mld2_report *) icmp6_hdr(skb); //
        icmp6h = icmp6_hdr(skb); //

        num = ntohs(mld2r->mld2r_ngrec);
        printk("[TEST]%s %u num1 = %u num2 = %u \n",
                  __func__, __LINE__,
                  ntohs(mld2r->mld2r_ngrec),
                  ntohs(icmp6h->icmp6_dataun.un_data16[1]));
        printk("[TEST]%s %u size1 = %lu size2 = %lu \n",
                  __func__, __LINE__,
                  sizeof(*mld2r),
                  sizeof(*icmp6h));

        len = skb_transport_offset(skb) + sizeof(*mld2r);


result:
[ 4932.345658] [TEST]br_ip6_multicast_mld2_report 2751 num1 = 2 num2 = 2
[ 4932.345665] [TEST]br_ip6_multicast_mld2_report 2752 size1 = 8 size2 = 8

The num1 and num2 are indeed equivalent.
Also, the size1 and size2 are equivalent.