linux-kernel - Re: [PATCH] staging: rtl8192u: Merge almost duplicate code

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:   Mon, 18 May 2020 16:25:23 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Pascal Terjan <pterjan@...gle.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8192u: Merge almost duplicate code

On Sun, May 17, 2020 at 09:25:05PM +0100, Pascal Terjan wrote:
> On Sun, 17 May 2020 at 17:58, Pascal Terjan <pterjan@...gle.com> wrote:
> >
> > This causes a change in behaviour:
> > - stats also get updated when reordering, this seems like it should be
> >   the case but those lines were commented out.
> > - sub_skb NULL check now happens early in both cases, previously it
> >   happened only after dereferencing it 12 times, so it may not actually
> >   be needed.
> >
> 
> Hi,
> I actually noticed the same duplicated code (and same late NULL check)
> in drivers/staging/rtl8192e/rtllib_rx.c
> drivers/staging/rtl8712/rtl8712_recv.c has only one copy of the code
> but with the late NULL check
> drivers/staging/rtl8188eu/core/rtw_recv.c has only one copy of the
> code and doesn't do any NULL check
> 
> Now I wonder how to proceed. The code is not great so it would not
> feel right to make it reusable.
> Should I continue improving it on this driver only first (maybe trying
> to reuse ieee80211_data_to_8023_exthdr from net/wireless/util.c for
> example)?

It looks like the NULL check could be removed, but it's also fine to
keep it so long as it's not after a NULL dereference.

Do whatever you have the energy to do...  It would be nice if people who
fix bugs in these Realtek drivers would check the other drivers as well
but for cleanups basically everyone just works on one driver only.

regards,
dan carpenter