2012-12-12 23:14:42

by Sven Wegener

[permalink] [raw]
Subject: [PATCH] NFSv4: Check for buffer length in __nfs4_get_acl_uncached

Commit 1f1ea6c "NFSv4: Fix buffer overflow checking in
__nfs4_get_acl_uncached" accidently dropped the checking for too small
result buffer length.

If someone uses getxattr on "system.nfs4_acl" on an NFSv4 mount
supporting ACLs, the ACL has not been cached and the buffer suplied is
too short, we still copy the complete ACL, resulting in kernel and user
space memory corruption.

Signed-off-by: Sven Wegener <[email protected]>
Cc: [email protected]
---
fs/nfs/nfs4proc.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

Resending, because it did not get any response.

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7bff871..f15be6b 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3831,8 +3831,13 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
goto out_free;
}
nfs4_write_cached_acl(inode, pages, res.acl_data_offset, res.acl_len);
- if (buf)
+ if (buf) {
+ if (res.acl_len > buflen) {
+ ret = -ERANGE;
+ goto out_free;
+ }
_copy_from_pages(buf, pages, res.acl_data_offset, res.acl_len);
+ }
out_ok:
ret = res.acl_len;
out_free:


2012-12-13 00:44:28

by Myklebust, Trond

[permalink] [raw]
Subject: RE: [PATCH] NFSv4: Check for buffer length in __nfs4_get_acl_uncached

PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1uZnMtb3duZXJAdmdl
ci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtbmZzLQ0KPiBvd25lckB2Z2VyLmtlcm5lbC5vcmdd
IE9uIEJlaGFsZiBPZiBTdmVuIFdlZ2VuZXINCj4gU2VudDogV2VkbmVzZGF5LCBEZWNlbWJlciAx
MiwgMjAxMiA2OjE1IFBNDQo+IFRvOiBNeWtsZWJ1c3QsIFRyb25kDQo+IENjOiBsaW51eC1uZnNA
dmdlci5rZXJuZWwub3JnOyBsaW51eC1rZXJuZWxAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6
IFtQQVRDSF0gTkZTdjQ6IENoZWNrIGZvciBidWZmZXIgbGVuZ3RoIGluDQo+IF9fbmZzNF9nZXRf
YWNsX3VuY2FjaGVkDQo+IA0KPiBDb21taXQgMWYxZWE2YyAiTkZTdjQ6IEZpeCBidWZmZXIgb3Zl
cmZsb3cgY2hlY2tpbmcgaW4NCj4gX19uZnM0X2dldF9hY2xfdW5jYWNoZWQiIGFjY2lkZW50bHkg
ZHJvcHBlZCB0aGUgY2hlY2tpbmcgZm9yIHRvbyBzbWFsbA0KPiByZXN1bHQgYnVmZmVyIGxlbmd0
aC4NCj4gDQo+IElmIHNvbWVvbmUgdXNlcyBnZXR4YXR0ciBvbiAic3lzdGVtLm5mczRfYWNsIiBv
biBhbiBORlN2NCBtb3VudA0KPiBzdXBwb3J0aW5nIEFDTHMsIHRoZSBBQ0wgaGFzIG5vdCBiZWVu
IGNhY2hlZCBhbmQgdGhlIGJ1ZmZlciBzdXBsaWVkIGlzIHRvbw0KPiBzaG9ydCwgd2Ugc3RpbGwg
Y29weSB0aGUgY29tcGxldGUgQUNMLCByZXN1bHRpbmcgaW4ga2VybmVsIGFuZCB1c2VyIHNwYWNl
DQo+IG1lbW9yeSBjb3JydXB0aW9uLg0KPiANCj4gU2lnbmVkLW9mZi1ieTogU3ZlbiBXZWdlbmVy
IDxzdmVuLndlZ2VuZXJAc3RlYWxlci5uZXQ+DQo+IENjOiBzdGFibGVAa2VybmVsLm9yZw0KPiAt
LS0NCj4gIGZzL25mcy9uZnM0cHJvYy5jIHwgNyArKysrKystDQo+ICAxIGZpbGUgY2hhbmdlZCwg
NiBpbnNlcnRpb25zKCspLCAxIGRlbGV0aW9uKC0pDQo+IA0KPiBSZXNlbmRpbmcsIGJlY2F1c2Ug
aXQgZGlkIG5vdCBnZXQgYW55IHJlc3BvbnNlLg0KDQpTb3JyeS4gSSd2ZSBhbHJlYWR5IGFwcGxp
ZWQgaXQgdG8gdGhlIG5mcy1mb3ItbmV4dCBicmFuY2ggb24gZ2l0LmxpbnV4LW5mcy5vcmcsIHNv
IGl0IHNob3VsZCBnbyBpbiBkdXJpbmcgdGhpcyBtZXJnZSB3aW5kb3cuDQoNCkNoZWVycw0KICBU
cm9uZA0K