Re: [ 26/48] udf: Avoid run away loop when partition table length is corrupted

On Tue 10-07-12 16:52:26, Ben Hutchings wrote:

On Mon, Jul 09, 2012 at 03:31:42PM +0100, Ben Hutchings wrote:

quoted

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Jan Kara <jack@suse.cz>

commit adee11b2085bee90bd8f4f52123ffb07882d6256 upstream.

Check provided length of partition table so that (possibly maliciously)
corrupted partition table cannot cause accessing data beyond current buffer.

Signed-off-by: Jan Kara <jack@suse.cz>
Signed-off-by: Ben Hutchings <redacted>
---
 fs/udf/super.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 9da6f4e..ce911f5 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c

[...]

quoted

@@ -1232,13 +1233,20 @@ static int udf_load_logicalvol(struct super_block *sb, sector_t block,
 		return 1;
 	BUG_ON(ident != TAG_IDENT_LVD);
 	lvd = (struct logicalVolDesc *)bh->b_data;
+	table_len = le32_to_cpu(lvd->mapTableLength);
+	if (sizeof(*lvd) + table_len > sb->s_blocksize) {

[...]

I don't think this is sufficient, unless there has been some prior
validation of lvd->mapTableLength.  On a 32-bit machine, the addition
may overflow.  The untrusted value has to be validated before doing
any arithmetic on it, e.g.:

	if (table_len > sb->s_blocksize - sizeof(*lv)) {

  Yeah, thanks for spotting this! I've queued the attached patch. I don't
find this really pressing so I'll push it in the next merge window. OK?

								Honza

-- 
Jan Kara [off-list ref]
SUSE Labs, CR