Skip to contentSkip to conversation

udf: fix uninit-value use in udf_get_fileshortad

PATCH In review Gianfranco Trad 2 participants 3 messages Sep 25, 2024 – Oct 3, 2024 1 message not in the archive UDF filesystem

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 20240925071355.t4w3thcfvfpou7gu@quack3

Gianfranco Trad

author
#1 lore raw

PATCH v2 1/1 udf: fix uninit-value use in udf_get_fileshortad

Check for overflow when computing alen in udf_current_aext to mitigate later uninit-value use in udf_get_fileshortad KMSAN bug[1]. After applying the patch reproducer did not trigger any issue[2].

[1] https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
[2] https://syzkaller.appspot.com/x/log.txt?x=10242227980000

Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
Tested-by [email protected]
Suggested-by Jan Kara <[email protected]>
Signed-off-by Gianfranco Trad <[email protected]>

---

Notes:
changes in v2:
- use check_add_overflow helper to check for overflow.

link to v1: https://lore.kernel.org/all/[email protected]/T/

 fs/udf/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/udf/inode.c +4 -3
index 4726a4d014b6..811a035b600f 100644
@@ -2215,9 +2215,10 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
 		if (!epos->offset)
 			epos->offset = sizeof(struct allocExtDesc);
 		ptr = epos->bh->b_data + epos->offset;
-		alen = sizeof(struct allocExtDesc) +
-			le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)->
-							lengthAllocDescs);
+		if (check_add_overflow(sizeof(struct allocExtDesc),
+					le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)
+						->lengthAllocDescs), &alen))
+			return -1;
 	}
 
 	switch (iinfo->i_alloc_type) {
Signature · 1 line
2.43.0
#2 lore raw
Quoted 4 lines · Gianfranco Trad

Check for overflow when computing alen in udf_current_aext to mitigate later uninit-value use in udf_get_fileshortad KMSAN bug[1]. After applying the patch reproducer did not trigger any issue[2].

Quoted 8 lines

[1] https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
[2] https://syzkaller.appspot.com/x/log.txt?x=10242227980000

Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
Tested-by [email protected]
Suggested-by Jan Kara <[email protected]>
Signed-off-by Gianfranco Trad <[email protected]>

Thanks. I've added the patch to my tree.

Honza

Quoted 32 lines

---

Notes:
changes in v2:
- use check_add_overflow helper to check for overflow.

link to v1: https://lore.kernel.org/all/[email protected]/T/

 fs/udf/inode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
fs/udf/inode.c +4 -3
index 4726a4d014b6..811a035b600f 100644
@@ -2215,9 +2215,10 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
 		if (!epos->offset)
 			epos->offset = sizeof(struct allocExtDesc);
 		ptr = epos->bh->b_data + epos->offset;
-		alen = sizeof(struct allocExtDesc) +
-			le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)->
-							lengthAllocDescs);
+		if (check_add_overflow(sizeof(struct allocExtDesc),
+					le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)
+						->lengthAllocDescs), &alen))
+			return -1;
 	}
 
 	switch (iinfo->i_alloc_type) {
Signature · 1 line
2.43.0
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
author
#3 lore raw
Quoted 14 lines · Jan Kara
Quoted 4 lines · Gianfranco Trad

Check for overflow when computing alen in udf_current_aext to mitigate later uninit-value use in udf_get_fileshortad KMSAN bug[1]. After applying the patch reproducer did not trigger any issue[2].

Quoted 8 lines

[1] https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
[2] https://syzkaller.appspot.com/x/log.txt?x=10242227980000

Reported-by [email protected]
Closes https://syzkaller.appspot.com/bug?extid=8901c4560b7ab5c2f9df
Tested-by [email protected]
Suggested-by Jan Kara <[email protected]>
Signed-off-by Gianfranco Trad <[email protected]>
Quoted 4 lines

Thanks. I've added the patch to my tree.

Honza

Thanks Jan. This was one of my first patches to the kernel. I will definitely remember it. Wish you a nice day :)

-- Gian

Quoted 32 lines
Quoted 32 lines

---

Notes:
changes in v2:
- use check_add_overflow helper to check for overflow.

link to v1: https://lore.kernel.org/all/[email protected]/T/

  fs/udf/inode.c | 7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)
fs/udf/inode.c +4 -3
index 4726a4d014b6..811a035b600f 100644
@@ -2215,9 +2215,10 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
  		if (!epos->offset)
  			epos->offset = sizeof(struct allocExtDesc);
  		ptr = epos->bh->b_data + epos->offset;
-		alen = sizeof(struct allocExtDesc) +
-			le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)->
-							lengthAllocDescs);
+		if (check_add_overflow(sizeof(struct allocExtDesc),
+					le32_to_cpu(((struct allocExtDesc *)epos->bh->b_data)
+						->lengthAllocDescs), &alen))
+			return -1;
  	}
 
  	switch (iinfo->i_alloc_type) {
Signature · 1 line
2.43.0