Skip to contentSkip to conversation

onenand: test before subtraction on unsigned

PATCH In review Roel Kluin 2 participants 3 messages Mar 4, 2009 Memory technology devices (MTD)

lore
Share & tools
lore

Roel Kluin

author
#0 lore raw

PATCH 1/1 onenand: test before subtraction on unsigned

len is unsigned so will wrap around when sizeof(struct otp_info) is greater than len.

---

drivers/mtd/onenand/onenand_base.c +3 -2
index 529af27..7c2ebe9 100644
@@ -2296,11 +2296,12 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
 		if (!action) {	/* OTP Info functions */
 			struct otp_info *otpinfo;
 
-			len -= sizeof(struct otp_info);
-			if (len <= 0) {
+			if (len <= sizeof(struct otp_info)) {
+				len = 0;
 				ret = -ENOSPC;
 				break;
 			}
+			len -= sizeof(struct otp_info);
 
 			otpinfo = (struct otp_info *) buf;
 			otpinfo->start = from;
#1 lore raw
Quoted 10 lines · Roel Kluin

len is unsigned so will wrap around when sizeof(struct otp_info) is greater than len.

---

drivers/mtd/onenand/onenand_base.c +0 -0
index 529af27..7c2ebe9 100644

drivers/mtd/onenand/onenand_base.c static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len, · line 2300

Quoted 7 lines
drivers/mtd/onenand/onenand_base.c +2 -2
@@ -2296,11 +2296,12 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
 		if (!action) {	/* OTP Info functions */
 			struct otp_info *otpinfo;
 
-			len -= sizeof(struct otp_info);
-			if (len <= 0) {
+			if (len <= sizeof(struct otp_info)) {
+				len = 0;

len is not used anymore, so no need to set it to zero.

drivers/mtd/onenand/onenand_base.c static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len, · line 2307

Quoted 7 lines
drivers/mtd/onenand/onenand_base.c +1 -0
 				ret = -ENOSPC;
 				break;
 			}
+			len -= sizeof(struct otp_info);
 
 			otpinfo = (struct otp_info *) buf;
 			otpinfo->start = from;

So is there somewhere that is passing a buffer too small for all the opt_info?

author
#2 lore raw
Quoted 11 lines · Adrian Hunter

Review

Quoted 8 lines · Roel Kluin

len is unsigned so will wrap around when sizeof(struct otp_info) is greater than
len.

(quoted diff) +2 -2
-            len -= sizeof(struct otp_info);
-            if (len <= 0) {
+            if (len <= sizeof(struct otp_info)) {
+                len = 0;

len is not used anymore, so no need to set it to zero.

Right, updated patch below.

Quoted 7 lines
Quoted 4 lines
                 ret = -ENOSPC;
                 break;
             }

+ len -= sizeof(struct otp_info);

So is there somewhere that is passing a buffer too small for all the opt_info?

I don't know, I found it by code inspection. ------------------------------>8-------------8<--------------------------------- len is unsigned so will wrap around when sizeof(struct otp_info) is greater than len.

---

drivers/mtd/onenand/onenand_base.c +2 -2
index 529af27..1219a18 100644
@@ -2296,11 +2296,11 @@ static int onenand_otp_walk(struct mtd_info *mtd, loff_t from, size_t len,
 		if (!action) {	/* OTP Info functions */
 			struct otp_info *otpinfo;
 
-			len -= sizeof(struct otp_info);
-			if (len <= 0) {
+			if (len <= sizeof(struct otp_info)) {
 				ret = -ENOSPC;
 				break;
 			}
+			len -= sizeof(struct otp_info);
 
 			otpinfo = (struct otp_info *) buf;
 			otpinfo->start = from;