[PATCH 0/2] patches addresses problem with memory corruption

When for raid0 metadata is updated outside mdmon it can happen that
memory corruption occurs due to too amount of memory is allocated
for imsm device structure.

1. imsm: FIX: mdmon crash during 2 raid0 arrays expansion
this patch is almost the same to sent earlier except 2 changes:
a) compilation problem
b) size should affect anchor size also
(added calculation changes extends space_needed variable also
this variable is used for anchor allocation)
so it replaces previous one.
2. imsm: FIX: sizeof_imsm_dev() can return too small value
size returned by sizeof_imsm_dev() describes minimum size of device
This is correct when both device maps has the same length
When device expands or shrinks we should return size that allows
for storing larger device information to avoid memory corruption.

BR
Adam

---

Adam Kwolek (2):
imsm: FIX: sizeof_imsm_dev() can return too small value
imsm: FIX: mdmon crash during 2 raid0 arrays expansion


super-intel.c | 20 +++++++++++++++-----
1 files changed, 15 insertions(+), 5 deletions(-)

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

[PATCH 2/2] imsm: FIX: sizeof_imsm_dev() can return too small value

sizeof_imsm_dev() should return value that can satisfy map operation
for 2 maps of size equal to bigger one.
If function reports too small value copy of bigger map can overwrite
other data in memory.

Signed-off-by: Adam Kwolek
---

super-intel.c | 13 +++++++++----
1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/super-intel.c b/super-intel.c
index 0c988d6..3c969c3 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -584,12 +584,17 @@ static size_t sizeof_imsm_dev(struct imsm_dev *dev, int migr_state)
{
size_t size = sizeof(*dev) - sizeof(struct imsm_map) +
sizeof_imsm_map(get_imsm_map(dev, 0));
+ int map_size = sizeof_imsm_map(get_imsm_map(dev, 0));
+
+ if (dev->vol.migr_state) {
+ int map1_size = sizeof_imsm_map(get_imsm_map(dev, 1));
+ if (map1_size > map_size)
+ map_size = map1_size;
+ }

/* migrating means an additional map */
- if (dev->vol.migr_state)
- size += sizeof_imsm_map(get_imsm_map(dev, 1));
- else if (migr_state)
- size += sizeof_imsm_map(get_imsm_map(dev, 0));
+ if ((dev->vol.migr_state) || (migr_state))
+ size += map_size;

return size;
}

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

RE: [PATCH 0/2] patches addresses problem with memory corruption

DQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogbGlu dXgtcmFpZC1vd25l
ckB2Z2VyLmtlcm5lbC5vcmcgW21haWx0bzpsaW51eC1yYWlkLQ0KPiBvd25l ckB2Z2VyLmtlcm5l
bC5vcmddIE9uIEJlaGFsZiBPZiBBZGFtIEt3b2xlaw0KPiBTZW50OiBNb25k YXksIEphbnVhcnkg
MzEsIDIwMTEgNDoyNSBQTQ0KPiBUbzogbmVpbGJAc3VzZS5kZQ0KPiBDYzog bGludXgtcmFpZEB2
Z2VyLmtlcm5lbC5vcmc7IFdpbGxpYW1zLCBEYW4gSjsgQ2llY2hhbm93c2tp LCBFZDsNCj4gTmV1
YmF1ZXIsIFdvamNpZWNoDQo+IFN1YmplY3Q6IFtQQVRDSCAwLzJdIHBhdGNo ZXMgYWRkcmVzc2Vz
IHByb2JsZW0gd2l0aCBtZW1vcnkgY29ycnVwdGlvbg0KPiANCj4gV2hlbiBm b3IgcmFpZDAgbWV0
YWRhdGEgaXMgdXBkYXRlZCBvdXRzaWRlIG1kbW9uIGl0IGNhbiBoYXBwZW4g dGhhdA0KPiBtZW1v
cnkgY29ycnVwdGlvbiBvY2N1cnMgZHVlIHRvIHRvbyBhbW91bnQgb2YgbWVt b3J5IGlzIGFsbG9j
YXRlZA0KPiBmb3IgaW1zbSBkZXZpY2Ugc3RydWN0dXJlLg0KPiANCj4gMS4g aW1zbTogRklYOiBt
ZG1vbiBjcmFzaCBkdXJpbmcgMiByYWlkMCBhcnJheXMgZXhwYW5zaW9uDQo+ ICAgdGhpcyBwYXRj
aCBpcyBhbG1vc3QgdGhlIHNhbWUgdG8gc2VudCBlYXJsaWVyIGV4Y2VwdCAy IGNoYW5nZXM6DQo+
ICAgIGEpIGNvbXBpbGF0aW9uIHByb2JsZW0NCj4gICAgYikgc2l6ZSBzaG91 bGQgYWZmZWN0IGFu
Y2hvciBzaXplIGFsc28NCj4gICAgICAgKGFkZGVkIGNhbGN1bGF0aW9uIGNo YW5nZXMgZXh0ZW5k
cyBzcGFjZV9uZWVkZWQgdmFyaWFibGUgYWxzbw0KPiAgICAgICAgdGhpcyB2 YXJpYWJsZSBpcyB1
c2VkIGZvciBhbmNob3IgYWxsb2NhdGlvbikNCj4gICBzbyBpdCByZXBsYWNl cyBwcmV2aW91cyBv
bmUuDQo+IDIuICBpbXNtOiBGSVg6IHNpemVvZl9pbXNtX2RldigpIGNhbiBy ZXR1cm4gdG9vIHNt
YWxsIHZhbHVlDQo+IAlzaXplIHJldHVybmVkIGJ5IHNpemVvZl9pbXNtX2Rl digpIGRlc2NyaWJl
cyBtaW5pbXVtIHNpemUgb2YNCj4gZGV2aWNlDQo+ICAgICAgICAgVGhpcyBp cyBjb3JyZWN0IHdo
ZW4gYm90aCBkZXZpY2UgbWFwcyBoYXMgdGhlIHNhbWUgbGVuZ3RoDQo+ICAg ICAgICAgV2hlbiBk
ZXZpY2UgZXhwYW5kcyBvciBzaHJpbmtzIHdlIHNob3VsZCByZXR1cm4gc2l6 ZSB0aGF0DQo+IGFs
bG93cw0KPiAgICAgICAgIGZvciBzdG9yaW5nIGxhcmdlciBkZXZpY2UgaW5m b3JtYXRpb24gdG8g
YXZvaWQgbWVtb3J5DQo+IGNvcnJ1cHRpb24uDQo+IA0KPiBCUg0KPiBBZGFt DQoNCmltc206IEZJ
WDogc2l6ZW9mX2ltc21fZGV2KCkgY2FuIHJldHVybiB0b28gc21hbGwgdmFs dWUNCglUaGlzIHNo
b3VsZCBiZSBhZGRyZXNzZWQgaW4gZGlmZmVyZW50IHdheSAoZml4IGluIGRp ZmZlcmVudCBwbGFj
ZSksIEkgaGF2ZSB0byB0aGluayBhYm91dCBpdC4uLg0KCXBsZWFzZSByZXZp ZXcvYXBwbHkgZmly
c3QgcGF0Y2ggb25seSAoaW1zbTogRklYOiBtZG1vbiBjcmFzaCBkdXJpbmcg MiByYWlkMCBhcnJh
eXMgZXhwYW5zaW9uKQ0KDQpCUg0KQWRhbQ0KDQo+IC0tLQ0KPiANCj4gQWRh bSBLd29sZWsgKDIp
Og0KPiAgICAgICBpbXNtOiBGSVg6IHNpemVvZl9pbXNtX2RldigpIGNhbiBy ZXR1cm4gdG9vIHNt
YWxsIHZhbHVlDQo+ICAgICAgIGltc206IEZJWDogbWRtb24gY3Jhc2ggZHVy aW5nIDIgcmFpZDAg
YXJyYXlzIGV4cGFuc2lvbg0KPiANCj4gDQo+ICBzdXBlci1pbnRlbC5jIHwg ICAyMCArKysrKysr
KysrKysrKystLS0tLQ0KPiAgMSBmaWxlcyBjaGFuZ2VkLCAxNSBpbnNlcnRp b25zKCspLCA1IGRl
bGV0aW9ucygtKQ0KPiANCj4gLS0NCj4gU2lnbmF0dXJlDQo+IC0tDQo+IFRv IHVuc3Vic2NyaWJl
IGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC1yYWlkIg0K
PiBpbg0KPiB0aGUgYm9keSBvZiBhIG1lc3NhZ2UgdG8gbWFqb3Jkb21vQHZn ZXIua2VybmVsLm9y
Zw0KPiBNb3JlIG1ham9yZG9tbyBpbmZvIGF0ICBodHRwOi8vdmdlci5rZXJu ZWwub3JnL21ham9y
ZG9tby1pbmZvLmh0bWwNCg==
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] patches addresses problem with memory corruption

On Mon, 31 Jan 2011 16:25:08 +0100 Adam Kwolek wrote:

> When for raid0 metadata is updated outside mdmon it can happen that
> memory corruption occurs due to too amount of memory is allocated
> for imsm device structure.
>
> 1. imsm: FIX: mdmon crash during 2 raid0 arrays expansion
> this patch is almost the same to sent earlier except 2 changes:
> a) compilation problem
> b) size should affect anchor size also
> (added calculation changes extends space_needed variable also
> this variable is used for anchor allocation)
> so it replaces previous one.
> 2. imsm: FIX: sizeof_imsm_dev() can return too small value
> size returned by sizeof_imsm_dev() describes minimum size of device
> This is correct when both device maps has the same length
> When device expands or shrinks we should return size that allows
> for storing larger device information to avoid memory corruption.
>

Both applied (replacing the previous one)..

Thanks.

NeilBrown


> BR
> Adam
>
> ---
>
> Adam Kwolek (2):
> imsm: FIX: sizeof_imsm_dev() can return too small value
> imsm: FIX: mdmon crash during 2 raid0 arrays expansion
>
>
> super-intel.c | 20 +++++++++++++++-----
> 1 files changed, 15 insertions(+), 5 deletions(-)
>

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

Re: [PATCH 0/2] patches addresses problem with memory corruption

On Mon, 31 Jan 2011 15:59:24 +0000 "Kwolek, Adam"
wrote:

>
>
> > -----Original Message-----
> > From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> > owner@vger.kernel.org] On Behalf Of Adam Kwolek
> > Sent: Monday, January 31, 2011 4:25 PM
> > To: neilb@suse.de
> > Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> > Neubauer, Wojciech
> > Subject: [PATCH 0/2] patches addresses problem with memory corruption
> >
> > When for raid0 metadata is updated outside mdmon it can happen that
> > memory corruption occurs due to too amount of memory is allocated
> > for imsm device structure.
> >
> > 1. imsm: FIX: mdmon crash during 2 raid0 arrays expansion
> > this patch is almost the same to sent earlier except 2 changes:
> > a) compilation problem
> > b) size should affect anchor size also
> > (added calculation changes extends space_needed variable also
> > this variable is used for anchor allocation)
> > so it replaces previous one.
> > 2. imsm: FIX: sizeof_imsm_dev() can return too small value
> > size returned by sizeof_imsm_dev() describes minimum size of
> > device
> > This is correct when both device maps has the same length
> > When device expands or shrinks we should return size that
> > allows
> > for storing larger device information to avoid memory
> > corruption.
> >
> > BR
> > Adam
>
> imsm: FIX: sizeof_imsm_dev() can return too small value
> This should be addressed in different way (fix in different place), I have to think about it...
> please review/apply first patch only (imsm: FIX: mdmon crash during 2 raid0 arrays expansion)
>

ahhh.. ok. Yes, I see your point.
The size adjustment has to happen at the point when we malloc.

I suspect we should get ride of the idea of always allocating enough space to
hold the migration info as well, as we don't really know in advance how big
that is going to be. Maybe we shoul always allocated (or re-allocated)
exactly how much we know we need now...

I had applied this patch, but I have now reverted it.
Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html