linux-kernel - Re: [PATCH 1/1] rapidio/rio_cm: avoid GFP

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20160916150801.24b13aed1984dd37de21592c@linux-foundation.org>
Date:   Fri, 16 Sep 2016 15:08:01 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Alexandre Bounine <alexandre.bounine@....com>
Cc:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] rapidio/rio_cm: avoid GFP_KERNEL in atomic context

On Thu, 15 Sep 2016 13:54:02 -0400 Alexandre Bounine <alexandre.bounine@....com> wrote:

> As reported by Alexey Khoroshilov <khoroshilov@...ras.ru>
> (see https://lkml.org/lkml/2016/9/9/737):
> riocm_send_close() is called from rio_cm_shutdown() under
> spin_lock_bh(idr_lock), but riocm_send_close() uses a GFP_KERNEL
> allocation.
> 
> Fix by taking riocm_send_close() outside of spinlock protected code.
> 
> --- a/drivers/rapidio/rio_cm.c
> +++ b/drivers/rapidio/rio_cm.c
> @@ -2242,17 +2242,31 @@ static int rio_cm_shutdown(struct notifier_block *nb, unsigned long code,
>  {
>  	struct rio_channel *ch;
>  	unsigned int i;
> +	LIST_HEAD(list);
>  
>  	riocm_debug(EXIT, ".");
>  
> +	/*
> +	 * If there are any channels left in connected state send
> +	 * close notification to the connection partner.
> +	 * First build a list of channels that require a closing
> +	 * notification because function riocm_send_close() should
> +	 * be called outside of spinlock protected code.
> +	 */
>  	spin_lock_bh(&idr_lock);
>  	idr_for_each_entry(&ch_idr, ch, i) {
> -		riocm_debug(EXIT, "close ch %d", ch->id);
> -		if (ch->state == RIO_CM_CONNECTED)
> -			riocm_send_close(ch);
> +		if (ch->state == RIO_CM_CONNECTED) {
> +			riocm_debug(EXIT, "close ch %d", ch->id);
> +			idr_remove(&ch_idr, ch->id);
> +			list_add(&ch->ch_node, &list);
> +		}
>  	}
>  	spin_unlock_bh(&idr_lock);
>  
> +	if (!list_empty(&list))
> +		list_for_each_entry(ch, &list, ch_node)
> +			riocm_send_close(ch);
> +
>  	return NOTIFY_DONE;
>  }

Fair enough.

Can we remove the !list_empty() test?

--- a/drivers/rapidio/rio_cm.c~rapidio-rio_cm-avoid-gfp_kernel-in-atomic-context-fix
+++ a/drivers/rapidio/rio_cm.c
@@ -2268,9 +2268,8 @@ static int rio_cm_shutdown(struct notifi
 	}
 	spin_unlock_bh(&idr_lock);
 
-	if (!list_empty(&list))
-		list_for_each_entry(ch, &list, ch_node)
-			riocm_send_close(ch);
+	list_for_each_entry(ch, &list, ch_node)
+		riocm_send_close(ch);
 
 	return NOTIFY_DONE;
 }
_