imsm: fix: does not allow to use invalid chunk size

PATCH Applied Przemyslaw Czarnowski 2 participants 4 messages Nov 25, 2011 – Dec 7, 2011 MD RAID

Share & tools

Przemyslaw Czarnowski

author

2011-11-25 12:12 UTC #0 lore raw

PATCH 1/1 imsm: fix: does not allow to use invalid chunk size

Only least significant bit of chunk size provided by user has been used in test with OROM capabilities. This way user could pass value which is not a power of 2.

Signed-off-by Przemyslaw Czarnowski <[email protected]>

---
 platform-intel.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

platform-intel.h +3 -1

index 6c094d7..99450ba 100644
@@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
 static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
 {
 	int fs = ffs(chunk);
+	int orom_chunk_bit;
 
 	if (!fs)
 		return 0;
 	fs--; /* bit num to bit index */
-	return !!(orom->sss & (1 << (fs - 1)));
+	orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
+	return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
 }

NeilBrown

Applied

2011-12-06 00:56 UTC #1 lore raw

Quoted 18 lines · On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski…

Only least significant bit of chunk size provided by user has been used in test with OROM capabilities. This way user could pass value which is not a power of 2.
Signed-off-by Przemyslaw Czarnowski <[email protected]>
---
 platform-intel.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
platform-intel.h +1 -0
index 6c094d7..99450ba 100644
@@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
 static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
 {
 	int fs = ffs(chunk);
+	int orom_chunk_bit;

platform-intel.h static inline int imsm_orom_has_raid5(const struct imsm_orom *orom) · line 136

Quoted 10 lines

platform-intel.h +2 -1

 
 	if (!fs)
 		return 0;
 	fs--; /* bit num to bit index */
-	return !!(orom->sss & (1 << (fs - 1)));
+	orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
+	return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
 }

applied, thanks.

However it feels a bit clumsy, though maybe that is just me. I would use a separate explicit test for "is a power of 2". ie.

  if (chunk & (chunk-1))
        return 0; /* not a power of 2 */
  fs = ffs(chunk);
  return !!(orom->sss & (1 << (fs-2)));

or something like that.

NeilBrown

Hawrylewicz Czarnowski, Przemyslaw

author

2011-12-06 15:49 UTC #2 lore raw

Quoted 43 lines

-----Original Message-----
From: NeilBrown [mailto:[email protected]]
Sent: Tuesday, December 06, 2011 1:56 AM
To: Hawrylewicz Czarnowski, Przemyslaw
Cc: [email protected]; Williams, Dan J; Labun, Marcin; Ciechanowski, Ed
Subject: Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
Quoted 19 lines · On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski…
Only least significant bit of chunk size provided by user has been used in test with OROM capabilities. This way user could pass value which is not a power of 2.
Signed-off-by: Przemyslaw Czarnowski
<[email protected]>
---
 platform-intel.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
platform-intel.h index renamed from platform-intel.h +0 -0
6c094d7..99450ba 100644
platform-intel.h +0 -0
@@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const
struct imsm_orom *orom) static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk) {
platform-intel.h
Quoted 10 lines
platform-intel.h +3 -1
 	int fs = ffs(chunk);
+	int orom_chunk_bit;
 
 	if (!fs)
 		return 0;
 	fs--; /* bit num to bit index */
-	return !!(orom->sss & (1 << (fs - 1)));
+	orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
+	return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
 }
applied, thanks.

Quoted 9 lines

However it feels a bit clumsy, though maybe that is just me. I would use a separate explicit test for "is a power of 2". ie.
  if (chunk & (chunk-1))
        return 0; /* not a power of 2 */
  fs = ffs(chunk);
  return !!(orom->sss & (1 << (fs-2)));

You're right,

Please get the one from the bottom of this email then... It is based on your input (~90%) so sign it up and ship:) Moreover previous one is not the right one I wanted to send out...

--
Best Regards,
Przemyslaw Hawrylewicz-Czarnowski

Quoted 3 lines

or something like that.
NeilBrown

Signature · 9 lines

Subject: [PATCH] fix: imsm: validate strip size - tuned up

Neil's proposal seems more reasonable and shows what is really going on
here.

Signed-off-by: Przemyslaw Czarnowski <[email protected]>
---
 platform-intel.h |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

platform-intel.h +3 -4

index 99450ba..c997f1b 100644
@@ -124,13 +124,12 @@ static inline int imsm_orom_has_raid5(const struct imsm_orom *orom)
 static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk)
 {
        int fs = ffs(chunk);
-       int orom_chunk_bit;
-
        if (!fs)
                return 0;
        fs--; /* bit num to bit index */
-       orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
-       return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
+       if (chunk & (chunk-1))
+               return 0; /* not a power of 2 */
+       return !!(orom->sss & (1 << (fs - 1)));
 }

Signature · 1 line

1.7.7

NeilBrown

Applied

2011-12-07 01:16 UTC #3 lore raw

Quoted 52 lines · Przemyslaw

Quoted 43 lines
-----Original Message-----
From: NeilBrown [mailto:[email protected]]
Sent: Tuesday, December 06, 2011 1:56 AM
To: Hawrylewicz Czarnowski, Przemyslaw
Cc: [email protected]; Williams, Dan J; Labun, Marcin; Ciechanowski, Ed
Subject: Re: [PATCH] imsm: fix: does not allow to use invalid chunk size
Quoted 19 lines · On Fri, 25 Nov 2011 13:12:04 +0100 Przemyslaw Czarnowski…
Only least significant bit of chunk size provided by user has been used in test with OROM capabilities. This way user could pass value which is not a power of 2.
Signed-off-by: Przemyslaw Czarnowski
<[email protected]>
---
 platform-intel.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
platform-intel.h index renamed from platform-intel.h +0 -0
6c094d7..99450ba 100644
platform-intel.h +0 -0
@@ -124,11 +124,13 @@ static inline int imsm_orom_has_raid5(const
struct imsm_orom *orom) static inline int imsm_orom_has_chunk(const struct imsm_orom *orom, int chunk) {
platform-intel.h static inline int imsm_orom_has_raid5(const struct imsm_orom *orom) · line 133
Quoted 10 lines
platform-intel.h +3 -1
 	int fs = ffs(chunk);
+	int orom_chunk_bit;
 
 	if (!fs)
 		return 0;
 	fs--; /* bit num to bit index */
-	return !!(orom->sss & (1 << (fs - 1)));
+	orom_chunk_bit = (orom->sss & (1 << (fs - 1)));
+	return orom_chunk_bit && 1 << orom_chunk_bit == chunk;
 }
applied, thanks.
Quoted 9 lines
However it feels a bit clumsy, though maybe that is just me. I would use a separate explicit test for "is a power of 2". ie.
  if (chunk & (chunk-1))
        return 0; /* not a power of 2 */
  fs = ffs(chunk);
  return !!(orom->sss & (1 << (fs-2)));

Quoted 4 lines

You're right,
Please get the one from the bottom of this email then... It is based on your input (~90%) so sign it up and ship:) Moreover previous one is not the right one I wanted to send out...

Applied, thanks.

NeilBrown