All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
Cc: Larry Finger <Larry.Finger@lwfinger.net>,
	Phillip Potter <phil@philpotter.co.uk>,
	linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context
Date: Tue, 8 Feb 2022 10:33:04 +0100	[thread overview]
Message-ID: <YgI40Dm/ar+IubIA@kroah.com> (raw)
In-Reply-To: <20220206225943.7848-1-fmdefrancesco@gmail.com>

On Sun, Feb 06, 2022 at 11:59:43PM +0100, Fabio M. De Francesco wrote:
> Use the GFP_ATOMIC flag of kzalloc() with two memory allocation in
> rtw_set_key(). This function is called while holding spinlocks and with
> disabled bottom halves, therefore it is not allowed to sleep. With the
> GFP_ATOMIC type flag, the allocation is high priority and cannot sleep.
> 
> This issue is detected by Smatch which emits the following warning:
> drivers/staging/r8188eu/core/rtw_pwrctrl.c:79 ips_leave() warn: sleeping in atomic context
> drivers/staging/r8188eu/core/rtw_pwrctrl.c:81 ips_leave() warn: sleeping in atomic context
> 
> The calls chain (in reverse order) is the following:
> 
> rtw_set_key()
> -> ips_leave()
> -> -> rtw_pwr_wakeup()
> -> -> -> rtw_set_802_11_disassociate()
> 
> The disabilitation of bottom halves and the acquisition of a spinlock is in
> rtw_set_802_11_disassociate().
> 
> After the changes, the post-commit hook output the following messages:
> 
> CHECK: Prefer kzalloc(sizeof(*pcmd)...) over kzalloc(sizeof(struct cmd_obj)...)
> +       pcmd = kzalloc(sizeof(struct cmd_obj), GFP_ATOMIC);
> 
> CHECK: Prefer kzalloc(sizeof(*psetkeyparm)...) over kzalloc(sizeof(struct setkey_parm)...)
> +       psetkeyparm = kzalloc(sizeof(struct setkey_parm), GFP_ATOMIC).
> 
> According to the above "CHECK[S]", use the preferred style in the first
> kzalloc().
> 
> Fixes: 79f712ea994d ("staging: r8188eu: Remove wrappers for kalloc() and kzalloc()")
> Fixes: 15865124feed ("staging: r8188eu: introduce new core dir for RTL8188eu driver")
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>  drivers/staging/r8188eu/core/rtw_mlme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/core/rtw_mlme.c b/drivers/staging/r8188eu/core/rtw_mlme.c
> index 038bddc361c3..860835e29b79 100644
> --- a/drivers/staging/r8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/r8188eu/core/rtw_mlme.c
> @@ -1600,12 +1600,12 @@ int rtw_set_key(struct adapter *adapter, struct security_priv *psecuritypriv, in
>  	struct mlme_priv		*pmlmepriv = &adapter->mlmepriv;
>  	int	res = _SUCCESS;
>  
> -	pcmd = kzalloc(sizeof(struct cmd_obj), GFP_KERNEL);
> +	pcmd = kzalloc(sizeof(*pcmd), GFP_ATOMIC);
>  	if (!pcmd) {
>  		res = _FAIL;  /* try again */
>  		goto exit;
>  	}
> -	psetkeyparm = kzalloc(sizeof(struct setkey_parm), GFP_KERNEL);
> +	psetkeyparm = kzalloc(sizeof(*psetkeyparm), GFP_ATOMIC);

You are making two different changes here.  Please do the first patch to
change the sizeof() change to fix up checkpatch, and then the second one
for the GFP_ATOMIC change so that if there is a problem with either of
them we can only revert the offending change.

thanks,

greg k-h

  reply	other threads:[~2022-02-08  9:33 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-06 22:59 [PATCH] staging: r8188eu: Use kzalloc() with GFP_ATOMIC in atomic context Fabio M. De Francesco
2022-02-08  9:33 ` Greg Kroah-Hartman [this message]
2022-02-08 10:22   ` Fabio M. De Francesco
  -- strict thread matches above, loose matches on Subject: below --
2021-11-01 11:41 Fabio M. De Francesco

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YgI40Dm/ar+IubIA@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Larry.Finger@lwfinger.net \
    --cc=fmdefrancesco@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=phil@philpotter.co.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.