linux-kernel - Re: [PATCH] staging: r8188eu: remove local BIT macro

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 21 Mar 2022 10:34:21 +0100
From:   Martin Kaiser <martin@...ser.cx>
To:     David Laight <David.Laight@...lab.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Michael Straube <straube.linux@...il.com>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] staging: r8188eu: remove local BIT macro

Hi David,

thanks for digging into this mess.

Thus wrote David Laight (David.Laight@...LAB.COM):

> From: Martin Kaiser <martin@...ser.cx>
> > Sent: 19 March 2022 18:04

> > The r8188eu driver defines a local BIT(x) macro. Remove this local macro
> > and use the one from include/linux/bits.h.

> > The global BIT macro returns an unsigned long value, the removed local
> > BIT macro used a signed int.

> > DYNAMIC_BB_DYNAMIC_TXPWR is defined as BIT(2), ~DYNAMIC_BB_DYNAMIC_TXPWR
> > is passed to Switch_DM_Func as a u32 parameter. We need a cast in this
> > case as ~DYNAMIC_BB_DYNAMIC_TXPWR is a 64-bit value on x86_64 systems.

> Hmmm....
> Why not fix the called function so that the caller doesn't
> need to do the invert.

When I look into this driver, it's challenging to fix one thing at a time
and not be caught up in all the other problems I discover along the way.

Would you be ok with removing the local BIT macro first?

It seems to me that this whole DYNAMIC_BB_DYNAMIC_TXPWR handling can go.
DYNAMIC_BB_DYNAMIC_TXPWR isn't checked anywhere. I'll submit this as a
separate patch.

With DYNAMIC_BB_DYNAMIC_TXPWR gone, ony three calls remain

  16   6723  drivers/staging/r8188eu/core/rtw_mlme_ext.c <<mlmeext_joinbss_event_callback>>
             Switch_DM_Func(padapter, DYNAMIC_ALL_FUNC_ENABLE, true);
  17   7124  drivers/staging/r8188eu/core/rtw_mlme_ext.c <<createbss_hdl>>
             Switch_DM_Func(padapter, DYNAMIC_FUNC_DISABLE, false);
  18   7393  drivers/staging/r8188eu/core/rtw_mlme_ext.c <<sitesurvey_cmd_hdl>>
             Switch_DM_Func(padapter, DYNAMIC_FUNC_DISABLE, false);

DYNAMIC_FUNC_DISABLE is indeed 0 and sets podmpriv->SupportAbility = 0,
DYNAMIC_ALL_FUNC_ENABLE sets podmpriv->SupportAbility = pdmpriv->InitODMFlag.

We can do this without dubious casts and intermediate functions...

Best regards,
Martin