diff mbox

pnfs-block: removing DM device maybe cause oops when call dev_remove

Message ID 514A5F32.2020603@cn.fujitsu.com (mailing list archive)
State New, archived
Headers show

Commit Message

fanchaoting March 21, 2013, 1:15 a.m. UTC 
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<fanchaoting@cn.fujitsu.com>
>>
>> ---
>>  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<fanchaoting@cn.fujitsu.com>
Reviewed-by: Trond Myklebust <Trond.Myklebust@netapp.com>

---
 fs/nfs/blocklayout/blocklayoutdm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

--
1.7.1




--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Trond Myklebust March 21, 2013, 1:40 a.m. UTC | #1 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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) {