On Fri, 2016-06-17 at 10:06 +0200, Torsten Hilbrich wrote: > The value bytes comes from the filesystem which is about to be > mounted. We cannot trust that the value is always in the range > we expect it to be. > > Check its value before using it to calculate the length for the > crc32_le call. It value must be larger (or equal) sumoff + 4 and > smaller than the remaining space in the block where the superblock > is stored (BLOCK_SIZE - sumoff - 4). > > This fixes a problem where the accidential mounting of an encrypted > volume caused a kernel panic. It had the correct magic value 0x3434 > at offset 0x406 by chance and the following 2 bytes were 0x01, 0x00 > (interpreted as s_bytes value with value 1). > > Signed-off-by: Torsten Hilbrich <torsten.hilbrich@xxxxxxxxxxx> > Tested-by: Torsten Hilbrich <torsten.hilbrich@xxxxxxxxxxx> > --- > fs/nilfs2/the_nilfs.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/nilfs2/the_nilfs.c b/fs/nilfs2/the_nilfs.c > index 69bd801..21f6b23 100644 > --- a/fs/nilfs2/the_nilfs.c > +++ b/fs/nilfs2/the_nilfs.c > @@ -438,18 +438,19 @@ static int nilfs_valid_sb(struct nilfs_super_block *sbp) > static unsigned char sum[4]; > const int sumoff = offsetof(struct nilfs_super_block, s_sum); > size_t bytes; > + size_t crc_offset = sumoff + 4; Hard-coded value looks very weird and not understandable, usually. Could you change hard-coded value on some declared constant with reasonable name or on sizeof(something)? Thanks, Vyacheslav Dubeyko. > u32 crc; > > if (!sbp || le16_to_cpu(sbp->s_magic) != NILFS_SUPER_MAGIC) > return 0; > bytes = le16_to_cpu(sbp->s_bytes); > - if (bytes > BLOCK_SIZE) > + if (bytes < crc_offset || bytes > BLOCK_SIZE - crc_offset) > return 0; > crc = crc32_le(le32_to_cpu(sbp->s_crc_seed), (unsigned char *)sbp, > sumoff); > crc = crc32_le(crc, sum, 4); > - crc = crc32_le(crc, (unsigned char *)sbp + sumoff + 4, > - bytes - sumoff - 4); > + crc = crc32_le(crc, (unsigned char *)sbp + crc_offset, > + bytes - crc_offset); > return crc == le32_to_cpu(sbp->s_sum); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
