| Message ID | 1335379523-31415-1-git-send-email-xi.wang@gmail.com |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2012-04-25 at 14:45 -0400, Xi Wang wrote: > `csize' is read from disk and thus needs validation. Otherwise a bogus > value 0xffffffff would turn the subsequent kmalloc(csize + 1, ...) into > kmalloc(0, ...), leading to out-of-bounds write. > > This patch limits `csize' to JFFS2_MAX_NAME_LEN, which is also used > in jffs2_symlink(). I think your commit message is a not general enough because it talks about 0xFFFFFFFF value, but there may be any other large value as well. I've added the following cause to the commit message and pushed both patches to l2-mtd.git, thanks! Please, verify. The clause: "Artem: we actually validate csize by checking CRC, so this 0xFFs cannot come from empty flash region. But I guess an attacker could feed JFFS2 an image with random csize value, including 0xFFs."
On Apr 29, 2012, at 11:44 AM, Artem Bityutskiy wrote: > > I think your commit message is a not general enough because it talks > about 0xFFFFFFFF value, but there may be any other large value as well. > I've added the following cause to the commit message and pushed both > patches to l2-mtd.git, thanks! Please, verify. > > The clause: > > "Artem: we actually validate csize by checking CRC, so this 0xFFs cannot > come from empty flash region. But I guess an attacker could feed JFFS2 > an image with random csize value, including 0xFFs." Looks good to me. Thanks! - xi
diff --git a/fs/jffs2/readinode.c b/fs/jffs2/readinode.c index dc0437e..9897f38 100644 --- a/fs/jffs2/readinode.c +++ b/fs/jffs2/readinode.c @@ -1266,6 +1266,12 @@ static int jffs2_do_read_inode_internal(struct jffs2_sb_info *c, /* Symlink's inode data is the target path. Read it and * keep in RAM to facilitate quick follow symlink * operation. */ + uint32_t csize = je32_to_cpu(latest_node->csize); + if (csize > JFFS2_MAX_NAME_LEN) { + mutex_unlock(&f->sem); + jffs2_do_clear_inode(c, f); + return -ENAMETOOLONG; + } f->target = kmalloc(je32_to_cpu(latest_node->csize) + 1, GFP_KERNEL); if (!f->target) { JFFS2_ERROR("can't allocate %d bytes of memory for the symlink target path cache\n", je32_to_cpu(latest_node->csize));
`csize' is read from disk and thus needs validation. Otherwise a bogus value 0xffffffff would turn the subsequent kmalloc(csize + 1, ...) into kmalloc(0, ...), leading to out-of-bounds write. This patch limits `csize' to JFFS2_MAX_NAME_LEN, which is also used in jffs2_symlink(). Signed-off-by: Xi Wang <xi.wang@gmail.com> Cc: Artem Bityutskiy <dedekind1@gmail.com> --- fs/jffs2/readinode.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)