2017-06-05 08:02:51

by Jia-Ju Bai

[permalink] [raw]
Subject: [PATCH] fs: nfs: Fix a sleep-in-atomic bug in nfs_access_add_cache

The driver may sleep under a rcu read lock, and function call path is:
nfs_permission (acquire the lock by rcu_read_lock)
nfs_do_access
nfs_access_add_cache
kmalloc(GFP_KERNEL) --> may sleep

To fix it, "GFP_KERNEL" is replaced with "GFP_ATOMIC".

Signed-off-by: Jia-Ju Bai <[email protected]>
---
fs/nfs/dir.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 32ccd77..7a074db 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -2333,7 +2333,7 @@ static void nfs_access_add_rbtree(struct inode *inode, struct nfs_access_entry *

void nfs_access_add_cache(struct inode *inode, struct nfs_access_entry *set)
{
- struct nfs_access_entry *cache = kmalloc(sizeof(*cache), GFP_KERNEL);
+ struct nfs_access_entry *cache = kmalloc(sizeof(*cache), GFP_ATOMIC);
if (cache == NULL)
return;
RB_CLEAR_NODE(&cache->rb_node);
--
1.7.9.5




2017-06-05 11:48:44

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] fs: nfs: Fix a sleep-in-atomic bug in nfs_access_add_cache

T24gTW9uLCAyMDE3LTA2LTA1IGF0IDE2OjA1ICswODAwLCBKaWEtSnUgQmFpIHdyb3RlOg0KPiBU
aGUgZHJpdmVyIG1heSBzbGVlcCB1bmRlciBhIHJjdSByZWFkIGxvY2ssIGFuZCBmdW5jdGlvbiBj
YWxsIHBhdGgNCj4gaXM6DQo+IG5mc19wZXJtaXNzaW9uIChhY3F1aXJlIHRoZSBsb2NrIGJ5IHJj
dV9yZWFkX2xvY2spDQo+IMKgIG5mc19kb19hY2Nlc3MNCj4gwqDCoMKgwqBuZnNfYWNjZXNzX2Fk
ZF9jYWNoZQ0KPiDCoMKgwqDCoMKgwqBrbWFsbG9jKEdGUF9LRVJORUwpIC0tPiBtYXkgc2xlZXAN
Cj4gDQo+IFRvIGZpeCBpdCwgIkdGUF9LRVJORUwiIGlzIHJlcGxhY2VkIHdpdGggIkdGUF9BVE9N
SUMiLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogSmlhLUp1IEJhaSA8YmFpamlhanUxOTkwQDE2My5j
b20+DQo+IC0tLQ0KPiDCoGZzL25mcy9kaXIuYyB8wqDCoMKgwqAyICstDQo+IMKgMSBmaWxlIGNo
YW5nZWQsIDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEv
ZnMvbmZzL2Rpci5jIGIvZnMvbmZzL2Rpci5jDQo+IGluZGV4IDMyY2NkNzcuLjdhMDc0ZGIgMTAw
NjQ0DQo+IC0tLSBhL2ZzL25mcy9kaXIuYw0KPiArKysgYi9mcy9uZnMvZGlyLmMNCj4gQEAgLTIz
MzMsNyArMjMzMyw3IEBAIHN0YXRpYyB2b2lkIG5mc19hY2Nlc3NfYWRkX3JidHJlZShzdHJ1Y3Qg
aW5vZGUNCj4gKmlub2RlLCBzdHJ1Y3QgbmZzX2FjY2Vzc19lbnRyeSAqDQo+IMKgDQo+IMKgdm9p
ZCBuZnNfYWNjZXNzX2FkZF9jYWNoZShzdHJ1Y3QgaW5vZGUgKmlub2RlLCBzdHJ1Y3QNCj4gbmZz
X2FjY2Vzc19lbnRyeSAqc2V0KQ0KPiDCoHsNCj4gLQlzdHJ1Y3QgbmZzX2FjY2Vzc19lbnRyeSAq
Y2FjaGUgPSBrbWFsbG9jKHNpemVvZigqY2FjaGUpLA0KPiBHRlBfS0VSTkVMKTsNCj4gKwlzdHJ1
Y3QgbmZzX2FjY2Vzc19lbnRyeSAqY2FjaGUgPSBrbWFsbG9jKHNpemVvZigqY2FjaGUpLA0KPiBH
RlBfQVRPTUlDKTsNCj4gwqAJaWYgKGNhY2hlID09IE5VTEwpDQo+IMKgCQlyZXR1cm47DQo+IMKg
CVJCX0NMRUFSX05PREUoJmNhY2hlLT5yYl9ub2RlKTsNCg0KVGhlIFJDVSBsb2NrZWQgY29kZXBh
dGggd2lsbCBub3QgZXZlciBoaXQgbmZzX2FjY2Vzc19hZGRfcmJ0cmVlKCkuIEl0DQpyZXR1cm5z
IHdpdGggYW4gZXJyb3IgY29kZSBvZiAtRUNISUxEIGFmdGVyIHRoZSB0ZXN0IG9mICJtYXlfYmxv
Y2siLg0KDQpDaGVlcnMNCiAgVHJvbmQNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51eCBORlMg
Y2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJpbWFyeWRh
dGEuY29tDQo=


2017-06-05 12:34:26

by Jia-Ju Bai

[permalink] [raw]
Subject: Re: [PATCH] fs: nfs: Fix a sleep-in-atomic bug in nfs_access_add_cache

On 06/05/2017 07:48 PM, Trond Myklebust wrote:
> On Mon, 2017-06-05 at 16:05 +0800, Jia-Ju Bai wrote:
>> The driver may sleep under a rcu read lock, and function call path
>> is:
>> nfs_permission (acquire the lock by rcu_read_lock)
>> nfs_do_access
>> nfs_access_add_cache
>> kmalloc(GFP_KERNEL) --> may sleep
>>
>> To fix it, "GFP_KERNEL" is replaced with "GFP_ATOMIC".
>>
>> Signed-off-by: Jia-Ju Bai<[email protected]>
>> ---
>> fs/nfs/dir.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
>> index 32ccd77..7a074db 100644
>> --- a/fs/nfs/dir.c
>> +++ b/fs/nfs/dir.c
>> @@ -2333,7 +2333,7 @@ static void nfs_access_add_rbtree(struct inode
>> *inode, struct nfs_access_entry *
>>
>> void nfs_access_add_cache(struct inode *inode, struct
>> nfs_access_entry *set)
>> {
>> - struct nfs_access_entry *cache = kmalloc(sizeof(*cache),
>> GFP_KERNEL);
>> + struct nfs_access_entry *cache = kmalloc(sizeof(*cache),
>> GFP_ATOMIC);
>> if (cache == NULL)
>> return;
>> RB_CLEAR_NODE(&cache->rb_node);
> The RCU locked codepath will not ever hit nfs_access_add_rbtree(). It
> returns with an error code of -ECHILD after the test of "may_block".
>
> Cheers
> Trond
Yes, I think you are right.

Thanks,
Jia-Ju Bai