Skip to contentSkip to conversation

md:Avoid write invalid address if read_seqretry returned true.

PATCH Applied majianpeng 2 participants 2 messages Nov 8, 2012 – Nov 19, 2012 MD RAID

lore
Share & tools
lore

majianpeng

author
#0 lore raw

PATCH 1/2 md:Avoid write invalid address if read_seqretry returned true.

Hi Neil,
I updated this patch.There are two place to changed
1:The previous patch omit the parameter 'p'
2:Using p[i] instead of *p++; bbp[i] instead *bbp++.It make the code looks better. After rewriting ,i tested and it looked good.

If read_seqretry returned true and bbp was changed, it will write invalid address which can cause some serious problem.

This bug was introduced by commit v3.0-rc7-130-g2699b67. So fix is suitable for 3.0.y thru 3.6.y.

Reported-by [email protected]
Tested-by [email protected]
Cc [email protected]
Signed-off-by Jianpeng Ma <[email protected]>
---
 drivers/md/md.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/md/md.c +2 -2
index 9ab768a..1f86c48 100644
@@ -1817,10 +1817,10 @@ retry:
 			memset(bbp, 0xff, PAGE_SIZE);
 
 			for (i = 0 ; i < bb->count ; i++) {
-				u64 internal_bb = *p++;
+				u64 internal_bb = p[i];
 				u64 store_bb = ((BB_OFFSET(internal_bb) << 10)
 						| BB_LEN(internal_bb));
-				*bbp++ = cpu_to_le64(store_bb);
+				bbp[i] = cpu_to_le64(store_bb);
 			}
 			bb->changed = 0;
 			if (read_seqretry(&bb->lock, seq))
Signature · 1 line
1.7.9.5
Applied
#1 lore raw
Quoted 28 lines · On Thu, 8 Nov 2012 08:56:27 +0800 majianpeng <[email protected]> wrote:

Hi Neil,
I updated this patch.There are two place to changed
1:The previous patch omit the parameter 'p'
2:Using p[i] instead of *p++; bbp[i] instead *bbp++.It make the code looks better. After rewriting ,i tested and it looked good.

If read_seqretry returned true and bbp was changed, it will write invalid address which can cause some serious problem.

This bug was introduced by commit v3.0-rc7-130-g2699b67. So fix is suitable for 3.0.y thru 3.6.y.

Reported-by [email protected]
Tested-by [email protected]
Cc [email protected]
Signed-off-by Jianpeng Ma <[email protected]>
---
 drivers/md/md.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/md/md.c +0 -0
index 9ab768a..1f86c48 100644
@@ -1817,10 +1817,10 @@ retry:
 			memset(bbp, 0xff, PAGE_SIZE);

drivers/md/md.c retry: · line 1826

Quoted 10 lines
drivers/md/md.c +2 -2
 			for (i = 0 ; i < bb->count ; i++) {
-				u64 internal_bb = *p++;
+				u64 internal_bb = p[i];
 				u64 store_bb = ((BB_OFFSET(internal_bb) << 10)
 						| BB_LEN(internal_bb));
-				*bbp++ = cpu_to_le64(store_bb);
+				bbp[i] = cpu_to_le64(store_bb);
 			}
 			bb->changed = 0;
 			if (read_seqretry(&bb->lock, seq))

Yes, that does look cleaner. Thanks.

Applied in place of the previous version.

Thanks,
NeilBrown