Patchwork fat_chksum(): fix access above array bounds

login
register
mail settings
Submitter Loïc Minier
Date Aug. 21, 2010, 10:47 p.m.
Message ID <1282430843-32308-1-git-send-email-loic.minier@linaro.org>
Download mbox | patch
Permalink /patch/62356/
State New
Headers show

Comments

Loïc Minier - Aug. 21, 2010, 10:47 p.m.
Signed-off-by: Loïc Minier <loic.minier@linaro.org>
---
 block/vvfat.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
Kevin Wolf - Aug. 23, 2010, 2:42 p.m.
Am 22.08.2010 00:47, schrieb Loïc Minier:
> Signed-off-by: Loïc Minier <loic.minier@linaro.org>

Thanks, applied to the block patch.

How did you find this one? From a quick look it seems that the pattern
that name is intentionally overflowed to access extension is still there
in some places. So if this caused some real bug, I think we'll have to
fix the other ones, too.

Kevin
Loïc Minier - Aug. 23, 2010, 2:56 p.m.
On Mon, Aug 23, 2010, Kevin Wolf wrote:
> How did you find this one? From a quick look it seems that the pattern
> that name is intentionally overflowed to access extension is still there
> in some places. So if this caused some real bug, I think we'll have to
> fix the other ones, too.

 Compiler found that one
 http://launchpadlibrarian.net/54142111/buildlog_ubuntu-maverick-armel.qemu-maemo_0.0~20100806%2Bd7a5eb1-0ubuntu1~linaro2_FAILEDTOBUILD.txt.gz

 but I didn't quote it because I'm at a loss as to why it detected it
 now and on armel only.  I think I just got one accidental build on a
 modern gcc-4.4 tree because of the timing of my copy of the packages.

 I guess I should build them more regularly under a recent gcc.

Patch

diff --git a/block/vvfat.c b/block/vvfat.c
index 6d61c2e..365332a 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -512,7 +512,7 @@  static inline uint8_t fat_chksum(const direntry_t* entry)
     for(i=0;i<11;i++) {
         unsigned char c;
 
-        c = (i <= 8) ? entry->name[i] : entry->extension[i-8];
+        c = (i < 8) ? entry->name[i] : entry->extension[i-8];
         chksum=(((chksum&0xfe)>>1)|((chksum&0x01)?0x80:0)) + c;
     }