Skip to contentSkip to conversation

Incorrect loop condition

PATCH In review Nahor 3 participants 4 messages Jun 4, 2009 – Jun 8, 2009 Memory technology devices (MTD)

lore
Share & tools
lore

Nahor

author
#0 lore raw

PATCH 1/1 Incorrect loop condition

Hi all,

Here is a small patch to fix an obvious typo.

If the file contains only a few bytes in the last page and the padding option is selected, the loop may exit early (readlen < meminfo.oobsize). Most of the time it will still work though because the code tries to read the whole OOB in one chunk.

On a similar issue, shouldn't the loop reading the input file exit with error if cnt == 0? The loop should alread exit when all the expected data is read. So if cnt == 0, it means that nandwrite didn't get all the data it wanted. The code then goes on as if everything was correctly read so it will write the data from the previous page. This is a security risk (even though the necessary conditions make it nearly impossible in practice).

Regards,
Nahor

nandwrite.c 2009-06-03 15:29:24.000000000 -0700 renamed from nandwrite.c 2009-06-01 18:03:55.000000000 -0700 +1 -1
@@ -531,7 +531,7 @@
               if (writeoob) {
                       int tinycnt = 0;
 
-                       while(tinycnt < readlen) {
+                       while(tinycnt < meminfo.oobsize) {
                               cnt = read(ifd, oobreadbuf + tinycnt,
meminfo.oobsize - tinycnt);
                               if (cnt == 0) { // EOF
                                       break;
#1 lore raw
Quoted 11 lines · Nahor
nandwrite.c 2009-06-03 15:29:24.000000000 -0700 renamed from nandwrite.c 2009-06-01 18:03:55.000000000 -0700 +1 -1
@@ -531,7 +531,7 @@
                if (writeoob) {
                        int tinycnt = 0;
 
-                       while(tinycnt < readlen) {
+                       while(tinycnt < meminfo.oobsize) {
                                cnt = read(ifd, oobreadbuf + tinycnt,
 meminfo.oobsize - tinycnt);
                                if (cnt == 0) { // EOF
Quoted 2 lines
                                        break;

This looks valid for me. Would you please, send a patch which I can apply, which would include your name and Signed-off-by signature?

Signature · 2 lines
Best regards,
Artem Bityutskiy (Битюцкий Артём)
#2 lore raw

Small patch to fix an obvious typo.

If the file contains only a few bytes in the last page and the --oob option is selected, the loop may exit early (readlen < meminfo.oobsize). Most of the time it will still work though because the code tries to read the whole OOB in one chunk.

nandwrite.c +1 -1
@@ -531,7 +531,7 @@ int main(int argc, char * const argv[])
 		if (writeoob) {
 			int tinycnt = 0;
 
-			while(tinycnt < readlen) {
+			while(tinycnt < meminfo.oobsize) {
 				cnt = read(ifd, oobreadbuf + tinycnt, meminfo.oobsize - tinycnt);
 				if (cnt == 0) { // EOF
 					break;
#3 lore raw
Quoted 6 lines · Jehan Bing

Small patch to fix an obvious typo.

If the file contains only a few bytes in the last page and the --oob option is selected, the loop may exit early (readlen < meminfo.oobsize). Most of the time it will still work though because the code tries to read the whole OOB in one chunk.

Pushed, thanks.

Signature · 2 lines
Best regards,
Artem Bityutskiy (Битюцкий Артём)