Re: uninitialized variable in if_mue.c
On Tue, Sep 18, 2018 at 08:40:24AM +0100, Ricardo Mestre wrote:
>
> Ouch, of course! Still not enough caffeine in the system!
>
> Unfortunately I don't have such a card to test it, but this is the way it's
> done
> everywhere else, the writes are always done unconditionally and we just need
> to
> ensure that the hash table is initialized to 0 so I prefer to keep the
> consistency.
Thanks for fixing it. ok kevlo@
> Index: if_mue.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_mue.c,v
> retrieving revision 1.4
> diff -u -p -u -r1.4 if_mue.c
> --- if_mue.c 15 Aug 2018 07:13:51 - 1.4
> +++ if_mue.c 18 Sep 2018 07:27:27 -
> @@ -1016,6 +1016,7 @@ mue_iff(struct mue_softc *sc)
> rxfilt = mue_csr_read(sc, reg);
> rxfilt &= ~(MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH |
> MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST);
> + memset(hashtbl, 0, sizeof(hashtbl));
> ifp->if_flags &= ~IFF_ALLMULTI;
>
> /* Always accept broadcast frames. */
> @@ -1028,9 +1029,6 @@ mue_iff(struct mue_softc *sc)
> rxfilt |= MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST;
> } else {
> rxfilt |= MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH;
> -
> - /* Clear hash table. */
> - memset(hashtbl, 0, sizeof(hashtbl));
>
> /* Now program new ones. */
> ETHER_FIRST_MULTI(step, ac, enm);
>
> On 09:06 Tue 18 Sep , Claudio Jeker wrote:
> > On Tue, Sep 18, 2018 at 07:55:43AM +0100, Ricardo Mestre wrote:
> > > Hi,
> > >
> > > In the case that a mue(4) device is put in promiscuous mode then hashtbl
> > > will
> > > be used uninitialized a little bit down the road so set it 0 like it's
> > > done in
> > > a lot of other devices. Coverity ID 1473316.
> > >
> > > OK?
> >
> > Please also remove the later memset(). There is no need to do it twice.
> > It would also be an option to not issue mue_dataport_write(MUE_DP_SEL_VHF)
> > if the hash is not used (e.g. moving that up into the else block.
> > Which ever option taken it needs to be tested on real hardware.
> >
> > --
> > :wq Claudio
>
>
Re: uninitialized variable in if_mue.c
> Date: Tue, 18 Sep 2018 07:55:43 +0100 > From: Ricardo Mestre > > Hi, > > In the case that a mue(4) device is put in promiscuous mode then hashtbl will > be used uninitialized a little bit down the road so set it 0 like it's done in > a lot of other devices. Coverity ID 1473316. > > OK? > > Index: if_mue.c > === > RCS file: /cvs/src/sys/dev/usb/if_mue.c,v > retrieving revision 1.4 > diff -u -p -u -r1.4 if_mue.c > --- if_mue.c 15 Aug 2018 07:13:51 - 1.4 > +++ if_mue.c 18 Sep 2018 06:47:54 - > @@ -1016,6 +1016,7 @@ mue_iff(struct mue_softc *sc) > rxfilt = mue_csr_read(sc, reg); > rxfilt &= ~(MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH | > MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST); > + memset(hashtbl, 0x00, sizeof(hashtbl)); > ifp->if_flags &= ~IFF_ALLMULTI; > > /* Always accept broadcast frames. */ > > Thare already is a memset call to clear the hash table in the else block. I think that should simply be moved up.
Re: uninitialized variable in if_mue.c
Ouch, of course! Still not enough caffeine in the system!
Unfortunately I don't have such a card to test it, but this is the way it's done
everywhere else, the writes are always done unconditionally and we just need to
ensure that the hash table is initialized to 0 so I prefer to keep the
consistency.
Index: if_mue.c
===
RCS file: /cvs/src/sys/dev/usb/if_mue.c,v
retrieving revision 1.4
diff -u -p -u -r1.4 if_mue.c
--- if_mue.c15 Aug 2018 07:13:51 - 1.4
+++ if_mue.c18 Sep 2018 07:27:27 -
@@ -1016,6 +1016,7 @@ mue_iff(struct mue_softc *sc)
rxfilt = mue_csr_read(sc, reg);
rxfilt &= ~(MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH |
MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST);
+ memset(hashtbl, 0, sizeof(hashtbl));
ifp->if_flags &= ~IFF_ALLMULTI;
/* Always accept broadcast frames. */
@@ -1028,9 +1029,6 @@ mue_iff(struct mue_softc *sc)
rxfilt |= MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST;
} else {
rxfilt |= MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH;
-
- /* Clear hash table. */
- memset(hashtbl, 0, sizeof(hashtbl));
/* Now program new ones. */
ETHER_FIRST_MULTI(step, ac, enm);
On 09:06 Tue 18 Sep , Claudio Jeker wrote:
> On Tue, Sep 18, 2018 at 07:55:43AM +0100, Ricardo Mestre wrote:
> > Hi,
> >
> > In the case that a mue(4) device is put in promiscuous mode then hashtbl
> > will
> > be used uninitialized a little bit down the road so set it 0 like it's done
> > in
> > a lot of other devices. Coverity ID 1473316.
> >
> > OK?
>
> Please also remove the later memset(). There is no need to do it twice.
> It would also be an option to not issue mue_dataport_write(MUE_DP_SEL_VHF)
> if the hash is not used (e.g. moving that up into the else block.
> Which ever option taken it needs to be tested on real hardware.
>
> --
> :wq Claudio
Re: uninitialized variable in if_mue.c
On Tue, Sep 18, 2018 at 07:55:43AM +0100, Ricardo Mestre wrote: > Hi, > > In the case that a mue(4) device is put in promiscuous mode then hashtbl will > be used uninitialized a little bit down the road so set it 0 like it's done in > a lot of other devices. Coverity ID 1473316. > > OK? Please also remove the later memset(). There is no need to do it twice. It would also be an option to not issue mue_dataport_write(MUE_DP_SEL_VHF) if the hash is not used (e.g. moving that up into the else block. Which ever option taken it needs to be tested on real hardware. > Index: if_mue.c > === > RCS file: /cvs/src/sys/dev/usb/if_mue.c,v > retrieving revision 1.4 > diff -u -p -u -r1.4 if_mue.c > --- if_mue.c 15 Aug 2018 07:13:51 - 1.4 > +++ if_mue.c 18 Sep 2018 06:47:54 - > @@ -1016,6 +1016,7 @@ mue_iff(struct mue_softc *sc) > rxfilt = mue_csr_read(sc, reg); > rxfilt &= ~(MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH | > MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST); > + memset(hashtbl, 0x00, sizeof(hashtbl)); > ifp->if_flags &= ~IFF_ALLMULTI; > > /* Always accept broadcast frames. */ > -- :wq Claudio
uninitialized variable in if_mue.c
Hi, In the case that a mue(4) device is put in promiscuous mode then hashtbl will be used uninitialized a little bit down the road so set it 0 like it's done in a lot of other devices. Coverity ID 1473316. OK? Index: if_mue.c === RCS file: /cvs/src/sys/dev/usb/if_mue.c,v retrieving revision 1.4 diff -u -p -u -r1.4 if_mue.c --- if_mue.c15 Aug 2018 07:13:51 - 1.4 +++ if_mue.c18 Sep 2018 06:47:54 - @@ -1016,6 +1016,7 @@ mue_iff(struct mue_softc *sc) rxfilt = mue_csr_read(sc, reg); rxfilt &= ~(MUE_RFE_CTL_PERFECT | MUE_RFE_CTL_MULTICAST_HASH | MUE_RFE_CTL_UNICAST | MUE_RFE_CTL_MULTICAST); + memset(hashtbl, 0x00, sizeof(hashtbl)); ifp->if_flags &= ~IFF_ALLMULTI; /* Always accept broadcast frames. */
