After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len. However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com --- fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
- if (p_cstring[i] != '\0') + if (p_cstring[min(i, len - 1)] != '\0') lossy |= NLS_NAME_OVERLEN;
*uniname = '\0'; --
On Mon, Oct 6, 2025 at 8:45 PM Jeongjun Park aha310510@gmail.com wrote:
Hi Jeongjun,
After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len. However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
I think we need to pass FSLABEL_MAX - 1 to exfat_nls_to_utf16, not FSLABEL_MAX. Can you check it and update the patch? Thanks.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com
fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
if (p_cstring[i] != '\0')
if (p_cstring[min(i, len - 1)] != '\0') lossy |= NLS_NAME_OVERLEN; *uniname = '\0';--
Hi Namjae,
Namjae Jeon linkinjeon@kernel.org wrote:
On Mon, Oct 6, 2025 at 8:45 PM Jeongjun Park aha310510@gmail.com wrote:
Hi Jeongjun,
After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len. However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
I think we need to pass FSLABEL_MAX - 1 to exfat_nls_to_utf16, not FSLABEL_MAX. Can you check it and update the patch?
If the only reason to change len to FSLABEL_MAX - 1 is to prevent out-of-bounds, this isn't a very appropriate solution.
Because the return value of exfat_convert_char_to_ucs2() can be greater than 1, even if len is set to FSLABEL_MAX - 1, i may still be FSLABEL_MAX when the loop ends. Therefore, checking the last byte of p_cstring with the min() function is essential to ensure out-of-bounds prevention.
Thanks.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com
fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
if (p_cstring[i] != '\0')
if (p_cstring[min(i, len - 1)] != '\0') lossy |= NLS_NAME_OVERLEN; *uniname = '\0';--
Regards, Jeongjun Park
Hello!
On Monday 06 October 2025 20:45:07 Jeongjun Park wrote:
After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len.
It is really possible to have "i" greater than len? Because I do not see from the code how such thing could happen.
I see only a case when i is equal to len (which is also overflow).
My understanding: while-loop condition ensures that i cannot be greater than len and i is increased by exfat_convert_char_to_ucs2() function which has upper bound of "len-i". So value of i can be increased maximally by (len-i) which could lead to maximal value of i to be just "len".
However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com
fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
- if (p_cstring[i] != '\0')
- if (p_cstring[min(i, len - 1)] != '\0')
What about "if (i < len)" condition instead?
The p_cstring is the nul term string and my understanding is that the "p_cstring[i] != '\0'" is checking that i is at position of strlen()+1. So should not be "if (i < len)" the same check without need to dereference the p_cstring?
lossy |= NLS_NAME_OVERLEN;*uniname = '\0';
Hi Pali
Pali Rohár pali@kernel.org wrote:
Hello!
On Monday 06 October 2025 20:45:07 Jeongjun Park wrote:
After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len.
It is really possible to have "i" greater than len? Because I do not see from the code how such thing could happen.
I see only a case when i is equal to len (which is also overflow).
My understanding: while-loop condition ensures that i cannot be greater than len and i is increased by exfat_convert_char_to_ucs2() function which has upper bound of "len-i". So value of i can be increased maximally by (len-i) which could lead to maximal value of i to be just "len".
However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com
fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
if (p_cstring[i] != '\0')
if (p_cstring[min(i, len - 1)] != '\0')What about "if (i < len)" condition instead?
The p_cstring is the nul term string and my understanding is that the "p_cstring[i] != '\0'" is checking that i is at position of strlen()+1. So should not be "if (i < len)" the same check without need to dereference the p_cstring?
Thank you for the detailed explanation! I misunderstood.
In summary, since the variable i can never be greater than len, we don't need to consider this case. Therefore, if i is less than len, we can determine that an nls loss has occurred.
I think that under normal nls conditions, i should be equal to len immediately after the while loop terminates, so changing the condition here to "if (i != len)" would be a better way to make this clear.
This way, we can check for an nls loss without dereferencing p_cstring, and we can clearly indicate that i should be equal to len when the while loop terminates. What do you think?
Regards, Jeongjun Park
On Thursday 09 October 2025 18:05:26 Jeongjun Park wrote:
Hi Pali
Pali Rohár pali@kernel.org wrote:
Hello!
On Monday 06 October 2025 20:45:07 Jeongjun Park wrote:
After the loop that converts characters to ucs2 ends, the variable i may be greater than or equal to len.
It is really possible to have "i" greater than len? Because I do not see from the code how such thing could happen.
I see only a case when i is equal to len (which is also overflow).
My understanding: while-loop condition ensures that i cannot be greater than len and i is increased by exfat_convert_char_to_ucs2() function which has upper bound of "len-i". So value of i can be increased maximally by (len-i) which could lead to maximal value of i to be just "len".
However, when checking whether the last byte of p_cstring is NULL, the variable i is used as is, resulting in an out-of-bounds read if i >= len.
Therefore, to prevent this, we need to modify the function to check whether i is less than len, and if i is greater than or equal to len, to check p_cstring[len - 1] byte.
Cc: stable@vger.kernel.org Reported-by: syzbot+98cc76a76de46b3714d4@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98cc76a76de46b3714d4 Fixes: 370e812b3ec1 ("exfat: add nls operations") Signed-off-by: Jeongjun Park aha310510@gmail.com
fs/exfat/nls.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/exfat/nls.c b/fs/exfat/nls.c index 8243d94ceaf4..a52f3494eb20 100644 --- a/fs/exfat/nls.c +++ b/fs/exfat/nls.c @@ -616,7 +616,7 @@ static int exfat_nls_to_ucs2(struct super_block *sb, unilen++; }
if (p_cstring[i] != '\0')
if (p_cstring[min(i, len - 1)] != '\0')What about "if (i < len)" condition instead?
The p_cstring is the nul term string and my understanding is that the "p_cstring[i] != '\0'" is checking that i is at position of strlen()+1. So should not be "if (i < len)" the same check without need to dereference the p_cstring?
Thank you for the detailed explanation! I misunderstood.
In summary, since the variable i can never be greater than len, we don't need to consider this case. Therefore, if i is less than len, we can determine that an nls loss has occurred.
I think that under normal nls conditions, i should be equal to len immediately after the while loop terminates, so changing the condition here to "if (i != len)" would be a better way to make this clear.
This way, we can check for an nls loss without dereferencing p_cstring, and we can clearly indicate that i should be equal to len when the while loop terminates. What do you think?
Regards, Jeongjun Park
Hello, yes, this is how I understood what is the code doing and how to simple fix this reported problem.
linux-stable-mirror@lists.linaro.org
-
Jeongjun Park -
Namjae Jeon -
Pali Rohár