Re: [PATCH net-next] rds: do not call ->conn_alloc with GFP_KERNEL
On (02/13/18 12:04), David Miller wrote: > > Let's not over-engineer this. For one thing, whatever allocation bits > came down from the callers, we are going to lose here. Ok, I was wondering how much of the sk_allocation we want to keep in the future, so I did this admittedly weird thing. I'll change it to the obvious and submit v3. --Sowmini
Re: [PATCH net-next] rds: do not call ->conn_alloc with GFP_KERNEL
From: Sowmini VaradhanDate: Mon, 12 Feb 2018 15:30:38 -0800 > diff --git a/net/rds/connection.c b/net/rds/connection.c > index 94e190f..d0f5889 100644 > --- a/net/rds/connection.c > +++ b/net/rds/connection.c > @@ -221,6 +221,8 @@ static void __rds_conn_path_init(struct rds_connection > *conn, > conn->c_path[i].cp_index = i; > } > rcu_read_lock(); > + gfp &= ~GFP_KERNEL; > + gfp |= GFP_ATOMIC; > if (rds_destroy_pending(conn)) > ret = -ENETDOWN; > else I'd never seen this kind of gfp masking before, so I did a grep around and the only cases I saw of this kind of usage were for things like GFP_DMA and such. I could not find one case that did it to convert a sleeping into a non- sleeping GFP mask. Let's not over-engineer this. For one thing, whatever allocation bits came down from the callers, we are going to lose here. So just pass straight GFP_ATOMIC into the routines below here instead of the 'gfp' variable. Thanks.
[PATCH net-next] rds: do not call ->conn_alloc with GFP_KERNEL
Commit ebeeb1ad9b8a ("rds: tcp: use rds_destroy_pending() to synchronize netns/module teardown and rds connection/workq management") adds an rcu read critical section to __rds_conn_create. The memory allocations in that critcal section need to use GFP_ATOMIC to avoid sleeping. This patch was verified with syzkaller reproducer. Reported-by: syzbot+a0564419941aaae3f...@syzkaller.appspotmail.com Signed-off-by: Sowmini VaradhanFixes: ebeeb1ad9b8a ("rds: tcp: use rds_destroy_pending() to synchronize netns/module teardown and rds connection/workq management") --- net/rds/connection.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/net/rds/connection.c b/net/rds/connection.c index 94e190f..d0f5889 100644 --- a/net/rds/connection.c +++ b/net/rds/connection.c @@ -221,6 +221,8 @@ static void __rds_conn_path_init(struct rds_connection *conn, conn->c_path[i].cp_index = i; } rcu_read_lock(); + gfp &= ~GFP_KERNEL; + gfp |= GFP_ATOMIC; if (rds_destroy_pending(conn)) ret = -ENETDOWN; else -- 1.7.1