linux-kernel - Re: [char-misc-next 1/2] mei: add check for offline bit in every register access

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]

Message-ID: <2023102137-chooser-canyon-06ff@gregkh>
Date:   Sat, 21 Oct 2023 23:05:38 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Tomas Winkler <tomas.winkler@...el.com>
Cc:     Alexander Usyskin <alexander.usyskin@...el.com>,
        Vitaly Lubart <vitaly.lubart@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [char-misc-next 1/2] mei: add check for offline bit in every
 register access

On Wed, Oct 18, 2023 at 01:10:23PM +0300, Tomas Winkler wrote:
> From: Vitaly Lubart <vitaly.lubart@...el.com>
> 
> Added check for offline in every register access function.
> When offline bit is set the driver should not access any mei hw.
> 
> Signed-off-by: Vitaly Lubart <vitaly.lubart@...el.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> ---
>  drivers/misc/mei/hw-me.c | 154 +++++++++++++++++++++++++++++++++++++--
>  1 file changed, 146 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/misc/mei/hw-me.c b/drivers/misc/mei/hw-me.c
> index d11a0740b47c96c33591e69a..565da19bb980c845bc2cf3ed 100644
> --- a/drivers/misc/mei/hw-me.c
> +++ b/drivers/misc/mei/hw-me.c
> @@ -58,6 +58,9 @@ static inline void mei_me_reg_write(const struct mei_me_hw *hw,
>   */
>  static inline u32 mei_me_mecbrw_read(const struct mei_device *dev)
>  {
> +	if (dev->dev->offline)
> +		return 0;
> +

What prevents this bit from being set right after you check for it?
Same for all of the checks in this patch, this feels like you are trying
to paper over a removal without proper locking and error handling.

thanks,

greg k-h