when pnfs block using device mapper,if umounting later,it maybe
cause oops. we apply "1 + sizeof(bl_umount_request)" memory for
msg->data, the memory maybe overflow when we do "memcpy(&dataptr
[sizeof(bl_msg)], &bl_umount_request, sizeof(bl_umount_request))",
because the size of bl_msg is more than 1 byte.
Signed-off-by: fanchaoting<[email protected]>
---
fs/nfs/blocklayout/blocklayoutdm.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/fs/nfs/blocklayout/blocklayoutdm.c b/fs/nfs/blocklayout/blocklayoutdm.c
index 737d839..8df9afa 100644
--- a/fs/nfs/blocklayout/blocklayoutdm.c
+++ b/fs/nfs/blocklayout/blocklayoutdm.c
@@ -55,7 +55,7 @@ static void dev_remove(struct net *net, dev_t dev)
bl_pipe_msg.bl_wq = &nn->bl_wq;
memset(msg, 0, sizeof(*msg));
- msg->data = kzalloc(1 + sizeof(bl_umount_request), GFP_NOFS);
+ msg->data = kzalloc(sizeof(bl_msg) + sizeof(bl_umount_request), GFP_NOFS);
if (!msg->data)
goto out;
--
1.7.1
Myklebust, Trond 写道:
> On Wed, 2013-03-20 at 16:01 +0800, fanchaoting wrote:
>> when pnfs block using device mapper,if umounting later,it maybe
>> cause oops. we apply "1 + sizeof(bl_umount_request)" memory for
>> msg->data, the memory maybe overflow when we do "memcpy(&dataptr
>> [sizeof(bl_msg)], &bl_umount_request, sizeof(bl_umount_request))",
>> because the size of bl_msg is more than 1 byte.
>>
>> Signed-off-by: fanchaoting<[email protected]>
>>
>> ---
>> fs/nfs/blocklayout/blocklayoutdm.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/nfs/blocklayout/blocklayoutdm.c b/fs/nfs/blocklayout/blocklayoutdm.c
>> index 737d839..8df9afa 100644
>> --- a/fs/nfs/blocklayout/blocklayoutdm.c
>> +++ b/fs/nfs/blocklayout/blocklayoutdm.c
>> @@ -55,7 +55,7 @@ static void dev_remove(struct net *net, dev_t dev)
>>
>> bl_pipe_msg.bl_wq = &nn->bl_wq;
>> memset(msg, 0, sizeof(*msg));
>> - msg->data = kzalloc(1 + sizeof(bl_umount_request), GFP_NOFS);
>> + msg->data = kzalloc(sizeof(bl_msg) + sizeof(bl_umount_request), GFP_NOFS);
>> if (!msg->data)
>> goto out;
>>
>
> Why not just move the calculation of msg->len, and then use msg->len in
> the above allocation?
>
yes, you are right, thanks for reviewing, here is the change patch.
when pnfs block using device mapper,if umounting later,it maybe
cause oops. we apply "1 + sizeof(bl_umount_request)" memory for
msg->data, the memory maybe overflow when we do "memcpy(&dataptr
[sizeof(bl_msg)], &bl_umount_request, sizeof(bl_umount_request))",
because the size of bl_msg is more than 1 byte.
Signed-off-by: fanchaoting<[email protected]>
Reviewed-by: Trond Myklebust <[email protected]>
---
fs/nfs/blocklayout/blocklayoutdm.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/blocklayout/blocklayoutdm.c b/fs/nfs/blocklayout/blocklayoutdm.c
index 737d839..6fc7b5c 100644
--- a/fs/nfs/blocklayout/blocklayoutdm.c
+++ b/fs/nfs/blocklayout/blocklayoutdm.c
@@ -55,7 +55,8 @@ static void dev_remove(struct net *net, dev_t dev)
bl_pipe_msg.bl_wq = &nn->bl_wq;
memset(msg, 0, sizeof(*msg));
- msg->data = kzalloc(1 + sizeof(bl_umount_request), GFP_NOFS);
+ msg->len = sizeof(bl_msg) + bl_msg.totallen;
+ msg->data = kzalloc(msg->len, GFP_NOFS);
if (!msg->data)
goto out;
@@ -66,7 +67,6 @@ static void dev_remove(struct net *net, dev_t dev)
memcpy(msg->data, &bl_msg, sizeof(bl_msg));
dataptr = (uint8_t *) msg->data;
memcpy(&dataptr[sizeof(bl_msg)], &bl_umount_request, sizeof(bl_umount_request));
- msg->len = sizeof(bl_msg) + bl_msg.totallen;
add_wait_queue(&nn->bl_wq, &wq);
if (rpc_queue_upcall(nn->bl_device_pipe, msg) < 0) {
--
1.7.1
On Wed, 2013-03-20 at 16:01 +0800, fanchaoting wrote:
> when pnfs block using device mapper,if umounting later,it maybe
> cause oops. we apply "1 + sizeof(bl_umount_request)" memory for
> msg->data, the memory maybe overflow when we do "memcpy(&dataptr
> [sizeof(bl_msg)], &bl_umount_request, sizeof(bl_umount_request))",
> because the size of bl_msg is more than 1 byte.
>
> Signed-off-by: fanchaoting<[email protected]>
>
> ---
> fs/nfs/blocklayout/blocklayoutdm.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/fs/nfs/blocklayout/blocklayoutdm.c b/fs/nfs/blocklayout/blocklayoutdm.c
> index 737d839..8df9afa 100644
> --- a/fs/nfs/blocklayout/blocklayoutdm.c
> +++ b/fs/nfs/blocklayout/blocklayoutdm.c
> @@ -55,7 +55,7 @@ static void dev_remove(struct net *net, dev_t dev)
>
> bl_pipe_msg.bl_wq = &nn->bl_wq;
> memset(msg, 0, sizeof(*msg));
> - msg->data = kzalloc(1 + sizeof(bl_umount_request), GFP_NOFS);
> + msg->data = kzalloc(sizeof(bl_msg) + sizeof(bl_umount_request), GFP_NOFS);
> if (!msg->data)
> goto out;
>
Why not just move the calculation of msg->len, and then use msg->len in
the above allocation?
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBsaW51eC1uZnMtb3duZXJAdmdl
ci5rZXJuZWwub3JnIFttYWlsdG86bGludXgtbmZzLW93bmVyQHZnZXIua2VybmVsLm9yZ10gT24g
QmVoYWxmIE9mDQo+IGZhbmNoYW90aW5nDQo+IFNlbnQ6IFdlZG5lc2RheSwgTWFyY2ggMjAsIDIw
MTMgNDowMSBQTQ0KPiBUbzogTXlrbGVidXN0LCBUcm9uZA0KPiBDYzogbGludXgtbmZzQHZnZXIu
a2VybmVsLm9yZw0KPiBTdWJqZWN0OiBbUEFUQ0hdIHBuZnMtYmxvY2s6IHJlbW92aW5nIERNIGRl
dmljZSBtYXliZSBjYXVzZSBvb3BzIHdoZW4gY2FsbCBkZXZfcmVtb3ZlDQo+IA0KPiB3aGVuIHBu
ZnMgYmxvY2sgdXNpbmcgZGV2aWNlIG1hcHBlcixpZiB1bW91bnRpbmcgbGF0ZXIsaXQgbWF5YmUN
Cj4gY2F1c2Ugb29wcy4gd2UgYXBwbHkgIjEgKyBzaXplb2YoYmxfdW1vdW50X3JlcXVlc3QpIiBt
ZW1vcnkgZm9yDQo+IG1zZy0+ZGF0YSwgdGhlIG1lbW9yeSBtYXliZSBvdmVyZmxvdyB3aGVuIHdl
IGRvICJtZW1jcHkoJmRhdGFwdHINCj4gW3NpemVvZihibF9tc2cpXSwgJmJsX3Vtb3VudF9yZXF1
ZXN0LCBzaXplb2YoYmxfdW1vdW50X3JlcXVlc3QpKSIsDQo+IGJlY2F1c2UgdGhlIHNpemUgb2Yg
YmxfbXNnIGlzIG1vcmUgdGhhbiAxIGJ5dGUuDQo+IA0KTmljZSBjYXRjaCEgSSB0aGluayB3ZSBk
aWRuJ3QgY3Jhc2ggYmVmb3JlIGp1c3QgYmVjYXVzZSBvZiBkYXRhIGFsaWdubWVudCAoYW5kIGx1
Y2sgZm9yIHN1cmUgOi0pLg0KDQpUaGFua3MsDQpUYW8NCj4gICAgU2lnbmVkLW9mZi1ieTogZmFu
Y2hhb3Rpbmc8ZmFuY2hhb3RpbmdAY24uZnVqaXRzdS5jb20+DQo+IA0KPiAtLS0NCj4gIGZzL25m
cy9ibG9ja2xheW91dC9ibG9ja2xheW91dGRtLmMgfCAgICAyICstDQo+ICAxIGZpbGVzIGNoYW5n
ZWQsIDEgaW5zZXJ0aW9ucygrKSwgMSBkZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9m
cy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRkbS5jIGIvZnMvbmZzL2Jsb2NrbGF5b3V0L2Js
b2NrbGF5b3V0ZG0uYw0KPiBpbmRleCA3MzdkODM5Li44ZGY5YWZhIDEwMDY0NA0KPiAtLS0gYS9m
cy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRkbS5jDQo+ICsrKyBiL2ZzL25mcy9ibG9ja2xh
eW91dC9ibG9ja2xheW91dGRtLmMNCj4gQEAgLTU1LDcgKzU1LDcgQEAgc3RhdGljIHZvaWQgZGV2
X3JlbW92ZShzdHJ1Y3QgbmV0ICpuZXQsIGRldl90IGRldikNCj4gDQo+ICAgICAgICAgYmxfcGlw
ZV9tc2cuYmxfd3EgPSAmbm4tPmJsX3dxOw0KPiAgICAgICAgIG1lbXNldChtc2csIDAsIHNpemVv
ZigqbXNnKSk7DQo+IC0gICAgICAgbXNnLT5kYXRhID0ga3phbGxvYygxICsgc2l6ZW9mKGJsX3Vt
b3VudF9yZXF1ZXN0KSwgR0ZQX05PRlMpOw0KPiArICAgICAgIG1zZy0+ZGF0YSA9IGt6YWxsb2Mo
c2l6ZW9mKGJsX21zZykgKyBzaXplb2YoYmxfdW1vdW50X3JlcXVlc3QpLCBHRlBfTk9GUyk7DQo+
ICAgICAgICAgaWYgKCFtc2ctPmRhdGEpDQo+ICAgICAgICAgICAgICAgICBnb3RvIG91dDsNCj4g
DQo+IC0tDQo+IDEuNy4xDQo+IA0KPiAtLQ0KPiBUbyB1bnN1YnNjcmliZSBmcm9tIHRoaXMgbGlz
dDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbGludXgtbmZzIiBpbg0KPiB0aGUgYm9keSBv
ZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZnZXIua2VybmVsLm9yZw0KPiBNb3JlIG1ham9yZG9t
byBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJuZWwub3JnL21ham9yZG9tby1pbmZvLmh0bWwNCg0K
T24gVGh1LCAyMDEzLTAzLTIxIGF0IDA5OjE1ICswODAwLCBmYW5jaGFvdGluZyB3cm90ZToNCj4g
TXlrbGVidXN0LCBUcm9uZCDlhpnpgZM6DQo+ID4gT24gV2VkLCAyMDEzLTAzLTIwIGF0IDE2OjAx
ICswODAwLCBmYW5jaGFvdGluZyB3cm90ZToNCj4gPj4gd2hlbiBwbmZzIGJsb2NrIHVzaW5nIGRl
dmljZSBtYXBwZXIsaWYgdW1vdW50aW5nIGxhdGVyLGl0IG1heWJlDQo+ID4+IGNhdXNlIG9vcHMu
IHdlIGFwcGx5ICIxICsgc2l6ZW9mKGJsX3Vtb3VudF9yZXF1ZXN0KSIgbWVtb3J5IGZvcg0KPiA+
PiBtc2ctPmRhdGEsIHRoZSBtZW1vcnkgbWF5YmUgb3ZlcmZsb3cgd2hlbiB3ZSBkbyAibWVtY3B5
KCZkYXRhcHRyDQo+ID4+IFtzaXplb2YoYmxfbXNnKV0sICZibF91bW91bnRfcmVxdWVzdCwgc2l6
ZW9mKGJsX3Vtb3VudF9yZXF1ZXN0KSkiLA0KPiA+PiBiZWNhdXNlIHRoZSBzaXplIG9mIGJsX21z
ZyBpcyBtb3JlIHRoYW4gMSBieXRlLg0KPiA+Pg0KPiA+PiAgICBTaWduZWQtb2ZmLWJ5OiBmYW5j
aGFvdGluZzxmYW5jaGFvdGluZ0Bjbi5mdWppdHN1LmNvbT4NCj4gPj4NCj4gPj4gLS0tDQo+ID4+
ICBmcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRkbS5jIHwgICAgMiArLQ0KPiA+PiAgMSBm
aWxlcyBjaGFuZ2VkLCAxIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb25zKC0pDQo+ID4+DQo+ID4+
IGRpZmYgLS1naXQgYS9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRkbS5jIGIvZnMvbmZz
L2Jsb2NrbGF5b3V0L2Jsb2NrbGF5b3V0ZG0uYw0KPiA+PiBpbmRleCA3MzdkODM5Li44ZGY5YWZh
IDEwMDY0NA0KPiA+PiAtLS0gYS9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRkbS5jDQo+
ID4+ICsrKyBiL2ZzL25mcy9ibG9ja2xheW91dC9ibG9ja2xheW91dGRtLmMNCj4gPj4gQEAgLTU1
LDcgKzU1LDcgQEAgc3RhdGljIHZvaWQgZGV2X3JlbW92ZShzdHJ1Y3QgbmV0ICpuZXQsIGRldl90
IGRldikNCj4gPj4NCj4gPj4gICAgICAgICBibF9waXBlX21zZy5ibF93cSA9ICZubi0+Ymxfd3E7
DQo+ID4+ICAgICAgICAgbWVtc2V0KG1zZywgMCwgc2l6ZW9mKCptc2cpKTsNCj4gPj4gLSAgICAg
ICBtc2ctPmRhdGEgPSBremFsbG9jKDEgKyBzaXplb2YoYmxfdW1vdW50X3JlcXVlc3QpLCBHRlBf
Tk9GUyk7DQo+ID4+ICsgICAgICAgbXNnLT5kYXRhID0ga3phbGxvYyhzaXplb2YoYmxfbXNnKSAr
IHNpemVvZihibF91bW91bnRfcmVxdWVzdCksIEdGUF9OT0ZTKTsNCj4gPj4gICAgICAgICBpZiAo
IW1zZy0+ZGF0YSkNCj4gPj4gICAgICAgICAgICAgICAgIGdvdG8gb3V0Ow0KPiA+Pg0KPiA+IA0K
PiA+IFdoeSBub3QganVzdCBtb3ZlIHRoZSBjYWxjdWxhdGlvbiBvZiBtc2ctPmxlbiwgYW5kIHRo
ZW4gdXNlIG1zZy0+bGVuIGluDQo+ID4gdGhlIGFib3ZlIGFsbG9jYXRpb24/DQo+ID4gDQo+IA0K
PiB5ZXMsIHlvdSBhcmUgcmlnaHQsIHRoYW5rcyBmb3IgcmV2aWV3aW5nLCBoZXJlIGlzIHRoZSBj
aGFuZ2UgcGF0Y2guDQo+IA0KPiANCj4gd2hlbiBwbmZzIGJsb2NrIHVzaW5nIGRldmljZSBtYXBw
ZXIsaWYgdW1vdW50aW5nIGxhdGVyLGl0IG1heWJlDQo+IGNhdXNlIG9vcHMuIHdlIGFwcGx5ICIx
ICsgc2l6ZW9mKGJsX3Vtb3VudF9yZXF1ZXN0KSIgbWVtb3J5IGZvcg0KPiBtc2ctPmRhdGEsIHRo
ZSBtZW1vcnkgbWF5YmUgb3ZlcmZsb3cgd2hlbiB3ZSBkbyAibWVtY3B5KCZkYXRhcHRyDQo+IFtz
aXplb2YoYmxfbXNnKV0sICZibF91bW91bnRfcmVxdWVzdCwgc2l6ZW9mKGJsX3Vtb3VudF9yZXF1
ZXN0KSkiLA0KPiBiZWNhdXNlIHRoZSBzaXplIG9mIGJsX21zZyBpcyBtb3JlIHRoYW4gMSBieXRl
Lg0KPiANCj4gU2lnbmVkLW9mZi1ieTogZmFuY2hhb3Rpbmc8ZmFuY2hhb3RpbmdAY24uZnVqaXRz
dS5jb20+DQo+IFJldmlld2VkLWJ5OiBUcm9uZCBNeWtsZWJ1c3QgPFRyb25kLk15a2xlYnVzdEBu
ZXRhcHAuY29tPg0KPiANCj4gLS0tDQo+ICBmcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2tsYXlvdXRk
bS5jIHwgICAgNCArKy0tDQo+ICAxIGZpbGVzIGNoYW5nZWQsIDIgaW5zZXJ0aW9ucygrKSwgMiBk
ZWxldGlvbnMoLSkNCj4gDQo+IGRpZmYgLS1naXQgYS9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2ts
YXlvdXRkbS5jIGIvZnMvbmZzL2Jsb2NrbGF5b3V0L2Jsb2NrbGF5b3V0ZG0uYw0KPiBpbmRleCA3
MzdkODM5Li42ZmM3YjVjIDEwMDY0NA0KPiAtLS0gYS9mcy9uZnMvYmxvY2tsYXlvdXQvYmxvY2ts
YXlvdXRkbS5jDQo+ICsrKyBiL2ZzL25mcy9ibG9ja2xheW91dC9ibG9ja2xheW91dGRtLmMNCj4g
QEAgLTU1LDcgKzU1LDggQEAgc3RhdGljIHZvaWQgZGV2X3JlbW92ZShzdHJ1Y3QgbmV0ICpuZXQs
IGRldl90IGRldikNCj4gDQo+ICAgICAgICAgYmxfcGlwZV9tc2cuYmxfd3EgPSAmbm4tPmJsX3dx
Ow0KPiAgICAgICAgIG1lbXNldChtc2csIDAsIHNpemVvZigqbXNnKSk7DQo+IC0gICAgICAgbXNn
LT5kYXRhID0ga3phbGxvYygxICsgc2l6ZW9mKGJsX3Vtb3VudF9yZXF1ZXN0KSwgR0ZQX05PRlMp
Ow0KPiArICAgICAgIG1zZy0+bGVuID0gc2l6ZW9mKGJsX21zZykgKyBibF9tc2cudG90YWxsZW47
DQo+ICsgICAgICAgbXNnLT5kYXRhID0ga3phbGxvYyhtc2ctPmxlbiwgR0ZQX05PRlMpOw0KPiAg
ICAgICAgIGlmICghbXNnLT5kYXRhKQ0KPiAgICAgICAgICAgICAgICAgZ290byBvdXQ7DQo+IA0K
PiBAQCAtNjYsNyArNjcsNiBAQCBzdGF0aWMgdm9pZCBkZXZfcmVtb3ZlKHN0cnVjdCBuZXQgKm5l
dCwgZGV2X3QgZGV2KQ0KPiAgICAgICAgIG1lbWNweShtc2ctPmRhdGEsICZibF9tc2csIHNpemVv
ZihibF9tc2cpKTsNCj4gICAgICAgICBkYXRhcHRyID0gKHVpbnQ4X3QgKikgbXNnLT5kYXRhOw0K
PiAgICAgICAgIG1lbWNweSgmZGF0YXB0cltzaXplb2YoYmxfbXNnKV0sICZibF91bW91bnRfcmVx
dWVzdCwgc2l6ZW9mKGJsX3Vtb3VudF9yZXF1ZXN0KSk7DQo+IC0gICAgICAgbXNnLT5sZW4gPSBz
aXplb2YoYmxfbXNnKSArIGJsX21zZy50b3RhbGxlbjsNCj4gDQo+ICAgICAgICAgYWRkX3dhaXRf
cXVldWUoJm5uLT5ibF93cSwgJndxKTsNCj4gICAgICAgICBpZiAocnBjX3F1ZXVlX3VwY2FsbChu
bi0+YmxfZGV2aWNlX3BpcGUsIG1zZykgPCAwKSB7DQoNCllvdXIgZW1haWwgY2xpZW50IGFwcGVh
cnMgdG8gaGF2ZSByZXBsYWNlZCB0aGUgdGFicyB3aXRoIHNwYWNlDQpjaGFyYWN0ZXJzLiBQbGVh
c2Ugbm90ZSB0aGF0IHRoZXJlIGFyZSBzdWdnZXN0aW9ucyBpbg0KbGludXgvRG9jdW1lbnRhdGlv
bi9lbWFpbC1jbGllbnRzLnR4dCBmb3IgaG93IHRvIGNvZXJjZSBUaHVuZGVyYmlyZCB0bw0KY29v
cGVyYXRlIHdoZW4geW91IGFyZSBzZW5kaW5nIHBhdGNoZXMuDQoNCkFueWhvdywgSSd2ZSBmaXhl
ZCB1cCB0aGUgd2hpdGVzcGFjZSBkYW1hZ2UsIGFuZCBoYXZlIGFkZGVkIGEgQ2M6DQpzdGFibGVA
dmdlci5rZXJuZWwub3JnIGJlZm9yZSBhcHBseWluZy4NCg0KVGhhbmtzIQ0KICBUcm9uZA0KLS0g
DQpUcm9uZCBNeWtsZWJ1c3QNCkxpbnV4IE5GUyBjbGllbnQgbWFpbnRhaW5lcg0KDQpOZXRBcHAN
ClRyb25kLk15a2xlYnVzdEBuZXRhcHAuY29tDQp3d3cubmV0YXBwLmNvbQ0K