2021-01-12 11:19:27

by lianzhi chang

[permalink] [raw]
Subject: [PATCH] udf: fix the problem that the disc content is not displayed

When the capacity of the disc is too large (assuming the 4.7G
specification), the disc (UDF file system) will be burned
multiple times in the windows (Multisession Usage). When the
remaining capacity of the CD is less than 300M (estimated
value, for reference only), open the CD in the Linux system,
the content of the CD is displayed as blank (the kernel will
say "No VRS found"). Windows can display the contents of the
CD normally.
Through analysis, in the "fs/udf/super.c": udf_check_vsd
function, the actual value of VSD_MAX_SECTOR_OFFSET may
be much larger than 0x800000. According to the current code
logic, it is found that the type of sbi->s_session is "__s32",
when the remaining capacity of the disc is less than 300M
(take a set of test values: sector=3154903040,
sbi->s_session=1540464, sb->s_blocksize_bits=11 ), the
calculation result of "sbi->s_session << sb->s_blocksize_bits"
will overflow. Therefore, it is necessary to convert the
type of s_session to "loff_t" (when udf_check_vsd starts,
assign a value to _sector, which is also converted in this
way), so that the result will not overflow, and then the
content of the disc can be displayed normally.

Signed-off-by: lianzhi chang <[email protected]>
---
fs/udf/super.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 5bef3a68395d..6c3069cd1321 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -757,7 +757,7 @@ static int udf_check_vsd(struct super_block *sb)

if (nsr > 0)
return 1;
- else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits) ==
+ else if (!bh && sector - ((loff_t)sbi->s_session << sb->s_blocksize_bits) ==
VSD_FIRST_SECTOR_OFFSET)
return -1;
else
--
2.20.1




2021-01-13 12:56:18

by Steve Magnani

[permalink] [raw]
Subject: Re: [PATCH] udf: fix the problem that the disc content is not displayed

On 2021-01-11 23:53, lianzhi chang wrote:
> When the capacity of the disc is too large (assuming the 4.7G
> specification), the disc (UDF file system) will be burned
> multiple times in the windows (Multisession Usage). When the
> remaining capacity of the CD is less than 300M (estimated
> value, for reference only), open the CD in the Linux system,
> the content of the CD is displayed as blank (the kernel will
> say "No VRS found"). Windows can display the contents of the
> CD normally.
> Through analysis, in the "fs/udf/super.c": udf_check_vsd
> function, the actual value of VSD_MAX_SECTOR_OFFSET may
> be much larger than 0x800000. According to the current code
> logic, it is found that the type of sbi->s_session is "__s32",
> when the remaining capacity of the disc is less than 300M
> (take a set of test values: sector=3154903040,
> sbi->s_session=1540464, sb->s_blocksize_bits=11 ), the
> calculation result of "sbi->s_session << sb->s_blocksize_bits"
> will overflow. Therefore, it is necessary to convert the
> type of s_session to "loff_t" (when udf_check_vsd starts,
> assign a value to _sector, which is also converted in this
> way), so that the result will not overflow, and then the
> content of the disc can be displayed normally.
>
> Signed-off-by: lianzhi chang <[email protected]>
> ---
> fs/udf/super.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 5bef3a68395d..6c3069cd1321 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -757,7 +757,7 @@ static int udf_check_vsd(struct super_block *sb)
>
> if (nsr > 0)
> return 1;
> - else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits) ==
> + else if (!bh && sector - ((loff_t)sbi->s_session <<
> sb->s_blocksize_bits) ==
> VSD_FIRST_SECTOR_OFFSET)
> return -1;
> else


Looks good. Perhaps consider factoring out the conversion (which also
occurs
earlier in the function) so that the complexity of this "else if" can be
reduced?

Reviewed-by: Steven J. Magnani <[email protected]>
------------------------------------------------------------------------
Steven J. Magnani "I claim this network for MARS!
Earthling, return my space modulator!"

#include <standard.disclaimer>

2021-01-14 03:23:49

by Steve Magnani

[permalink] [raw]
Subject: Re: [PATCH] udf: fix the problem that the disc content is not displayed

On 2021-01-13 20:51, 常廉志 wrote:

> On 2021-01-11 23:53, lianzhi chang wrote:
>
>>> When the capacity of the disc is too large (assuming the 4.7G
>>> specification), the disc (UDF file system) will be burned
>>> multiple times in the windows (Multisession Usage). When the
>>> remaining capacity of the CD is less than 300M (estimated
>>> value, for reference only), open the CD in the Linux system,
>>> the content of the CD is displayed as blank (the kernel will
>>> say "No VRS found"). Windows can display the contents of the
>>> CD normally.
>>> Through analysis, in the "fs/udf/super.c": udf_check_vsd
>>> function, the actual value of VSD_MAX_SECTOR_OFFSET may
>>> be much larger than 0x800000. According to the current code
>> l>ogic, it is found that the type of sbi->s_session is "__s32",
>>> when the remaining capacity of the disc is less than 300M
>>> (take a set of test values: sector=3154903040,
>>> sbi->s_session=1540464, sb->s_blocksize_bits=11 ), the
>>> calculation result of "sbi->s_session << sb->s_blocksize_bits"
>>> will overflow. Therefore, it is necessary to convert the
>>> type of s_session to "loff_t" (when udf_check_vsd starts,
>>> assign a value to _sector, which is also converted in this
>>> way), so that the result will not overflow, and then the
>>> content of the disc can be displayed normally.
>>>
>>> Signed-off-by: lianzhi chang <[email protected]>
>>> ---
>>> fs/udf/super.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/fs/udf/super.c b/fs/udf/super.c
>>> index 5bef3a68395d..6c3069cd1321 100644
>>> --- a/fs/udf/super.c
>>> +++ b/fs/udf/super.c
>>> @@ -757,7 +757,7 @@ static int udf_check_vsd(struct super_block *sb)
>>>
>>> if (nsr > 0)
>>> return 1;
>>> - else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits)
>>> ==
>>> + else if (!bh && sector - ((loff_t)sbi->s_session <<
>>> sb->s_blocksize_bits) ==
>>> VSD_FIRST_SECTOR_OFFSET)
>>> return -1;
>>> else
>>
>>
>> Looks good. Perhaps consider factoring out the conversion (which also
>> occurs
>> earlier in the function) so that the complexity of this "else if" can
>> be
>> reduced?
>>
>
>> Reviewed-by: Steven J. Magnani <magnani@xxxxxxxx>
>
> Thank you very much! So, which one of the following methods do you
> think is better:
>
> (1) Change the type of s_session in struct udf_sb_info to __s64. If you
> modify this way, it may involve some memory copy problems of the
> structure, and there are more modifications.
>
> (2) Definition: loff_t sector_offset=sbi->s_session <<
> sb->s_blocksize_bits, and then put sector_offset into the "else if"
> statement.
>
> (3) Or is there any other better way?

I had #2 in mind.
------------------------------------------------------------------------
Steven J. Magnani "I claim this network for MARS!
Earthling, return my space modulator!"
#include <standard.disclaimer>

2021-01-14 06:34:40

by lianzhi chang

[permalink] [raw]
Subject: Re: [PATCH] udf: fix the problem that the disc content is not displayed

On 2021-01-13 20:51, 常廉志 wrote:

>> On 2021-01-11 23:53, lianzhi chang wrote:
>>
>>>> When the capacity of the disc is too large (assuming the 4.7G
>>>> specification), the disc (UDF file system) will be burned
>>>> multiple times in the windows (Multisession Usage). When the
>>>> remaining capacity of the CD is less than 300M (estimated
>>>> value, for reference only), open the CD in the Linux system,
>>>> the content of the CD is displayed as blank (the kernel will
>>>> say "No VRS found"). Windows can display the contents of the
>>>> CD normally.
>>>> Through analysis, in the "fs/udf/super.c": udf_check_vsd
>>>> function, the actual value of VSD_MAX_SECTOR_OFFSET may
>>>> be much larger than 0x800000. According to the current code
>>> l>ogic, it is found that the type of sbi->s_session is "__s32",
>>>> when the remaining capacity of the disc is less than 300M
>>>> (take a set of test values: sector=3154903040,
>>>> sbi->s_session=1540464, sb->s_blocksize_bits=11 ), the
>>>> calculation result of "sbi->s_session << sb->s_blocksize_bits"
>>>> will overflow. Therefore, it is necessary to convert the
>>>> type of s_session to "loff_t" (when udf_check_vsd starts,
>>>> assign a value to _sector, which is also converted in this
>>>> way), so that the result will not overflow, and then the
>>>> content of the disc can be displayed normally.
>>>>
>>> Signed-off-by: lianzhi chang <[email protected]>
>>>> ---
>>>> fs/udf/super.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/udf/super.c b/fs/udf/super.c
>>>> index 5bef3a68395d..6c3069cd1321 100644
>>>> --- a/fs/udf/super.c
>>>> +++ b/fs/udf/super.c
>>>> @@ -757,7 +757,7 @@ static int udf_check_vsd(struct super_block *sb)
>>>>
>>>> if (nsr > 0)
>>>> return 1;
>>>> - else if (!bh && sector - (sbi->s_session << sb->s_blocksize_bits)
>>>> ==
>>>> + else if (!bh && sector - ((loff_t)sbi->s_session <<
>>>> sb->s_blocksize_bits) ==
>>>> VSD_FIRST_SECTOR_OFFSET)
>>>> return -1;
>>>> else
>>>
>>>
>>> Looks good. Perhaps consider factoring out the conversion (which also
>>> occurs
>>> earlier in the function) so that the complexity of this "else if" can
>>> be
>>> reduced?
>>>
>>
>>> Reviewed-by: Steven J. Magnani <magnani@xxxxxxxx>
>>
>> Thank you very much! So, which one of the following methods do you
>> think is better:
>>
>> (1) Change the type of s_session in struct udf_sb_info to __s64. If you
>> modify this way, it may involve some memory copy problems of the
>> structure, and there are more modifications.
>>
>> (2) Definition: loff_t sector_offset=sbi->s_session <<
>> sb->s_blocksize_bits, and then put sector_offset into the "else if"
>> statement.
>>
>> (3) Or is there any other better way?

>I had #2 in mind.
>------------------------------------------------------------------------
> Steven J. Magnani "I claim this network for MARS!

Thank you very much for your suggestion, I will submit a new patch