linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] udf: fix the problem that the disc content is not displayed
@ 2021-01-12  5:53 lianzhi chang
  2021-01-13 12:51 ` Steve Magnani
  0 siblings, 1 reply; 9+ messages in thread
From: lianzhi chang @ 2021-01-12  5:53 UTC (permalink / raw)
  To: jack; +Cc: linux-kernel, 282827961, lianzhi chang

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 <changlianzhi@uniontech.com>
---
 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




^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
  2021-01-12  5:53 [PATCH] udf: fix the problem that the disc content is not displayed lianzhi chang
@ 2021-01-13 12:51 ` Steve Magnani
       [not found]   ` <1400033179.660265.1610592703732.JavaMail.xmail@bj-wm-cp-9>
  0 siblings, 1 reply; 9+ messages in thread
From: Steve Magnani @ 2021-01-13 12:51 UTC (permalink / raw)
  To: lianzhi chang; +Cc: jack, linux-kernel, 282827961

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 <changlianzhi@uniontech.com>
> ---
>  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@ieee.org>
------------------------------------------------------------------------
  Steven J. Magnani               "I claim this network for MARS!
                                   Earthling, return my space modulator!"

  #include <standard.disclaimer>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
  2021-01-14  3:21     ` Steve Magnani
@ 2021-01-13 22:25       ` changlianzhi
  0 siblings, 0 replies; 9+ messages in thread
From: changlianzhi @ 2021-01-13 22:25 UTC (permalink / raw)
  To: magnani; +Cc: jack, linux-kernel, 282827961

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 <changlianzhi@uniontech.com>
>>>> ---
>>>> 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



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
       [not found]   ` <1400033179.660265.1610592703732.JavaMail.xmail@bj-wm-cp-9>
@ 2021-01-14  3:21     ` Steve Magnani
  2021-01-13 22:25       ` changlianzhi
  0 siblings, 1 reply; 9+ messages in thread
From: Steve Magnani @ 2021-01-14  3:21 UTC (permalink / raw)
  To: 常廉志; +Cc: jack, linux-kernel, 282827961

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 <changlianzhi@uniontech.com>
>>> ---
>>> 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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
  2021-01-14 13:26 lianzhi chang
@ 2021-01-14 17:15 ` Jan Kara
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Kara @ 2021-01-14 17:15 UTC (permalink / raw)
  To: lianzhi chang; +Cc: jack, magnani, linux-kernel, 282827961

On Thu 14-01-21 21:26:15, 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 <changlianzhi@uniontech.com>

There was no need to resend the patch (I've fixed up the problem locally -
it was easy enough) but thanks anyway :).

								Honza

> ---
>  fs/udf/super.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 5bef3a68395d..f2ff98f393fb 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -705,6 +705,7 @@ static int udf_check_vsd(struct super_block *sb)
>  	struct buffer_head *bh = NULL;
>  	int nsr = 0;
>  	struct udf_sb_info *sbi;
> +	loff_t sector_offset;
>  
>  	sbi = UDF_SB(sb);
>  	if (sb->s_blocksize < sizeof(struct volStructDesc))
> @@ -712,7 +713,8 @@ static int udf_check_vsd(struct super_block *sb)
>  	else
>  		sectorsize = sb->s_blocksize;
>  
> -	sector += (((loff_t)sbi->s_session) << sb->s_blocksize_bits);
> +	sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits;
> +	sector += sector_offset;
>  
>  	udf_debug("Starting at sector %u (%lu byte sectors)\n",
>  		  (unsigned int)(sector >> sb->s_blocksize_bits),
> @@ -757,8 +759,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) ==
> -			VSD_FIRST_SECTOR_OFFSET)
> +	else if (!bh && sector - sector_offset == VSD_FIRST_SECTOR_OFFSET)
>  		return -1;
>  	else
>  		return 0;
> -- 
> 2.20.1
> 
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] udf: fix the problem that the disc content is not displayed
@ 2021-01-14 13:26 lianzhi chang
  2021-01-14 17:15 ` Jan Kara
  0 siblings, 1 reply; 9+ messages in thread
From: lianzhi chang @ 2021-01-14 13:26 UTC (permalink / raw)
  To: jack, magnani; +Cc: linux-kernel, 282827961, lianzhi chang

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 <changlianzhi@uniontech.com>
---
 fs/udf/super.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 5bef3a68395d..f2ff98f393fb 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -705,6 +705,7 @@ static int udf_check_vsd(struct super_block *sb)
 	struct buffer_head *bh = NULL;
 	int nsr = 0;
 	struct udf_sb_info *sbi;
+	loff_t sector_offset;
 
 	sbi = UDF_SB(sb);
 	if (sb->s_blocksize < sizeof(struct volStructDesc))
@@ -712,7 +713,8 @@ static int udf_check_vsd(struct super_block *sb)
 	else
 		sectorsize = sb->s_blocksize;
 
-	sector += (((loff_t)sbi->s_session) << sb->s_blocksize_bits);
+	sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits;
+	sector += sector_offset;
 
 	udf_debug("Starting at sector %u (%lu byte sectors)\n",
 		  (unsigned int)(sector >> sb->s_blocksize_bits),
@@ -757,8 +759,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) ==
-			VSD_FIRST_SECTOR_OFFSET)
+	else if (!bh && sector - sector_offset == VSD_FIRST_SECTOR_OFFSET)
 		return -1;
 	else
 		return 0;
-- 
2.20.1




^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
  2021-01-14  7:57 lianzhi chang
  2021-01-14 10:41 ` Jan Kara
@ 2021-01-14 11:06 ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2021-01-14 11:06 UTC (permalink / raw)
  To: lianzhi chang, magnani, jack
  Cc: kbuild-all, clang-built-linux, linux-kernel, 282827961, lianzhi chang

[-- Attachment #1: Type: text/plain, Size: 10666 bytes --]

Hi lianzhi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.11-rc3 next-20210114]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/lianzhi-chang/udf-fix-the-problem-that-the-disc-content-is-not-displayed/20210114-161012
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 65f0d2414b7079556fbbcc070b3d1c9f9587606d
config: mips-randconfig-r026-20210114 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 6077d55381a6aa3e947ef7abdc36a7515c598c8a)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install mips cross compiling tool for clang build
        # apt-get install binutils-mips-linux-gnu
        # https://github.com/0day-ci/linux/commit/76a7c356fa5bfe473a3d7c15bda5922c902a33f7
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review lianzhi-chang/udf-fix-the-problem-that-the-disc-content-is-not-displayed/20210114-161012
        git checkout 76a7c356fa5bfe473a3d7c15bda5922c902a33f7
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/linux/atomic.h:7:
   arch/mips/include/asm/atomic.h:257:1: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
   ATOMIC_SIP_OP(atomic, int, subu, ll, sc)
   ^
   arch/mips/include/asm/atomic.h:251:7: note: expanded from macro 'ATOMIC_SIP_OP'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
   In file included from fs/udf/super.c:41:
   In file included from fs/udf/udfdecl.h:10:
   In file included from include/linux/fs.h:6:
   In file included from include/linux/wait_bit.h:8:
   In file included from include/linux/wait.h:9:
   In file included from include/linux/spinlock.h:51:
   In file included from include/linux/preempt.h:78:
   In file included from ./arch/mips/include/generated/asm/preempt.h:1:
   In file included from include/asm-generic/preempt.h:5:
   In file included from include/linux/thread_info.h:56:
   In file included from arch/mips/include/asm/thread_info.h:16:
   In file included from arch/mips/include/asm/processor.h:14:
   In file included from include/linux/atomic.h:7:
   arch/mips/include/asm/atomic.h:261:1: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
   ATOMIC_SIP_OP(atomic64, s64, dsubu, lld, scd)
   ^
   arch/mips/include/asm/atomic.h:251:7: note: expanded from macro 'ATOMIC_SIP_OP'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
   In file included from fs/udf/super.c:41:
   In file included from fs/udf/udfdecl.h:10:
   In file included from include/linux/fs.h:6:
   In file included from include/linux/wait_bit.h:8:
   In file included from include/linux/wait.h:9:
   In file included from include/linux/spinlock.h:59:
   In file included from include/linux/lockdep.h:14:
   In file included from include/linux/smp.h:15:
   In file included from include/linux/smp_types.h:5:
   include/linux/llist.h:237:9: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
           return xchg(&head->first, NULL);
                  ^
   arch/mips/include/asm/cmpxchg.h:102:7: note: expanded from macro 'xchg'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
   In file included from fs/udf/super.c:41:
   In file included from fs/udf/udfdecl.h:10:
   In file included from include/linux/fs.h:6:
   In file included from include/linux/wait_bit.h:8:
   In file included from include/linux/wait.h:9:
   In file included from include/linux/spinlock.h:59:
   In file included from include/linux/lockdep.h:27:
   include/linux/debug_locks.h:17:9: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
           return xchg(&debug_locks, 0);
                  ^
   arch/mips/include/asm/cmpxchg.h:102:7: note: expanded from macro 'xchg'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
   In file included from fs/udf/super.c:41:
   In file included from fs/udf/udfdecl.h:12:
   In file included from include/linux/buffer_head.h:14:
   In file included from include/linux/pagemap.h:8:
   In file included from include/linux/mm.h:33:
   In file included from include/linux/pgtable.h:6:
   arch/mips/include/asm/pgtable.h:202:3: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
                   cmpxchg64(&buddy->pte, 0, _PAGE_GLOBAL);
                   ^
   arch/mips/include/asm/cmpxchg.h:220:2: note: expanded from macro 'cmpxchg64'
           cmpxchg((ptr), (o), (n));                                       \
           ^
   arch/mips/include/asm/cmpxchg.h:194:7: note: expanded from macro 'cmpxchg'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
   In file included from fs/udf/super.c:41:
   In file included from fs/udf/udfdecl.h:12:
   In file included from include/linux/buffer_head.h:14:
   In file included from include/linux/pagemap.h:8:
   In file included from include/linux/mm.h:33:
   In file included from include/linux/pgtable.h:6:
   arch/mips/include/asm/pgtable.h:202:3: warning: converting the result of '<<' to a boolean always evaluates to true [-Wtautological-constant-compare]
   arch/mips/include/asm/cmpxchg.h:220:2: note: expanded from macro 'cmpxchg64'
           cmpxchg((ptr), (o), (n));                                       \
           ^
   arch/mips/include/asm/cmpxchg.h:204:7: note: expanded from macro 'cmpxchg'
           if (!__SYNC_loongson3_war)                                      \
                ^
   arch/mips/include/asm/sync.h:147:34: note: expanded from macro '__SYNC_loongson3_war'
   # define __SYNC_loongson3_war   (1 << 31)
                                      ^
>> fs/udf/super.c:716:64: error: extraneous ')' before ';'
           sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits);
                                                                         ^
   12 warnings and 1 error generated.


vim +716 fs/udf/super.c

   693	
   694	/*
   695	 * Check Volume Structure Descriptors (ECMA 167 2/9.1)
   696	 * We also check any "CD-ROM Volume Descriptor Set" (ECMA 167 2/8.3.1)
   697	 * @return   1 if NSR02 or NSR03 found,
   698	 *	    -1 if first sector read error, 0 otherwise
   699	 */
   700	static int udf_check_vsd(struct super_block *sb)
   701	{
   702		struct volStructDesc *vsd = NULL;
   703		loff_t sector = VSD_FIRST_SECTOR_OFFSET;
   704		int sectorsize;
   705		struct buffer_head *bh = NULL;
   706		int nsr = 0;
   707		struct udf_sb_info *sbi;
   708		loff_t sector_offset;
   709	
   710		sbi = UDF_SB(sb);
   711		if (sb->s_blocksize < sizeof(struct volStructDesc))
   712			sectorsize = sizeof(struct volStructDesc);
   713		else
   714			sectorsize = sb->s_blocksize;
   715	
 > 716		sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits);
   717		sector += sector_offset;
   718	
   719		udf_debug("Starting at sector %u (%lu byte sectors)\n",
   720			  (unsigned int)(sector >> sb->s_blocksize_bits),
   721			  sb->s_blocksize);
   722		/* Process the sequence (if applicable). The hard limit on the sector
   723		 * offset is arbitrary, hopefully large enough so that all valid UDF
   724		 * filesystems will be recognised. There is no mention of an upper
   725		 * bound to the size of the volume recognition area in the standard.
   726		 *  The limit will prevent the code to read all the sectors of a
   727		 * specially crafted image (like a bluray disc full of CD001 sectors),
   728		 * potentially causing minutes or even hours of uninterruptible I/O
   729		 * activity. This actually happened with uninitialised SSD partitions
   730		 * (all 0xFF) before the check for the limit and all valid IDs were
   731		 * added */
   732		for (; !nsr && sector < VSD_MAX_SECTOR_OFFSET; sector += sectorsize) {
   733			/* Read a block */
   734			bh = udf_tread(sb, sector >> sb->s_blocksize_bits);
   735			if (!bh)
   736				break;
   737	
   738			vsd = (struct volStructDesc *)(bh->b_data +
   739						      (sector & (sb->s_blocksize - 1)));
   740			nsr = identify_vsd(vsd);
   741			/* Found NSR or end? */
   742			if (nsr) {
   743				brelse(bh);
   744				break;
   745			}
   746			/*
   747			 * Special handling for improperly formatted VRS (e.g., Win10)
   748			 * where components are separated by 2048 bytes even though
   749			 * sectors are 4K
   750			 */
   751			if (sb->s_blocksize == 4096) {
   752				nsr = identify_vsd(vsd + 1);
   753				/* Ignore unknown IDs... */
   754				if (nsr < 0)
   755					nsr = 0;
   756			}
   757			brelse(bh);
   758		}
   759	
   760		if (nsr > 0)
   761			return 1;
   762		else if (!bh && sector - sector_offset == VSD_FIRST_SECTOR_OFFSET)
   763			return -1;
   764		else
   765			return 0;
   766	}
   767	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37708 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] udf: fix the problem that the disc content is not displayed
  2021-01-14  7:57 lianzhi chang
@ 2021-01-14 10:41 ` Jan Kara
  2021-01-14 11:06 ` kernel test robot
  1 sibling, 0 replies; 9+ messages in thread
From: Jan Kara @ 2021-01-14 10:41 UTC (permalink / raw)
  To: lianzhi chang; +Cc: magnani, jack, linux-kernel, 282827961

On Thu 14-01-21 15:57:41, 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 <changlianzhi@uniontech.com>
> ---
>  fs/udf/super.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 5bef3a68395d..f2ff98f393fb 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -705,6 +705,7 @@ static int udf_check_vsd(struct super_block *sb)
>  	struct buffer_head *bh = NULL;
>  	int nsr = 0;
>  	struct udf_sb_info *sbi;
> +	loff_t sector_offset;
>  
>  	sbi = UDF_SB(sb);
>  	if (sb->s_blocksize < sizeof(struct volStructDesc))
> @@ -712,7 +713,8 @@ static int udf_check_vsd(struct super_block *sb)
>  	else
>  		sectorsize = sb->s_blocksize;
>  
> -	sector += (((loff_t)sbi->s_session) << sb->s_blocksize_bits);
> +	sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits);

There's imbalanced parentheses here, I'll fix it up on commit. Otherwise
the fix looks good to me. Thanks!

								Honza

> +	sector += sector_offset;
>  
>  	udf_debug("Starting at sector %u (%lu byte sectors)\n",
>  		  (unsigned int)(sector >> sb->s_blocksize_bits),
> @@ -757,8 +759,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) ==
> -			VSD_FIRST_SECTOR_OFFSET)
> +	else if (!bh && sector - sector_offset == VSD_FIRST_SECTOR_OFFSET)
>  		return -1;
>  	else
>  		return 0;
> -- 
> 2.20.1
> 
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] udf: fix the problem that the disc content is not displayed
@ 2021-01-14  7:57 lianzhi chang
  2021-01-14 10:41 ` Jan Kara
  2021-01-14 11:06 ` kernel test robot
  0 siblings, 2 replies; 9+ messages in thread
From: lianzhi chang @ 2021-01-14  7:57 UTC (permalink / raw)
  To: magnani, jack; +Cc: linux-kernel, 282827961, lianzhi chang

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 <changlianzhi@uniontech.com>
---
 fs/udf/super.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/udf/super.c b/fs/udf/super.c
index 5bef3a68395d..f2ff98f393fb 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -705,6 +705,7 @@ static int udf_check_vsd(struct super_block *sb)
 	struct buffer_head *bh = NULL;
 	int nsr = 0;
 	struct udf_sb_info *sbi;
+	loff_t sector_offset;
 
 	sbi = UDF_SB(sb);
 	if (sb->s_blocksize < sizeof(struct volStructDesc))
@@ -712,7 +713,8 @@ static int udf_check_vsd(struct super_block *sb)
 	else
 		sectorsize = sb->s_blocksize;
 
-	sector += (((loff_t)sbi->s_session) << sb->s_blocksize_bits);
+	sector_offset = (loff_t)sbi->s_session << sb->s_blocksize_bits);
+	sector += sector_offset;
 
 	udf_debug("Starting at sector %u (%lu byte sectors)\n",
 		  (unsigned int)(sector >> sb->s_blocksize_bits),
@@ -757,8 +759,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) ==
-			VSD_FIRST_SECTOR_OFFSET)
+	else if (!bh && sector - sector_offset == VSD_FIRST_SECTOR_OFFSET)
 		return -1;
 	else
 		return 0;
-- 
2.20.1




^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-01-14 17:16 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-12  5:53 [PATCH] udf: fix the problem that the disc content is not displayed lianzhi chang
2021-01-13 12:51 ` Steve Magnani
     [not found]   ` <1400033179.660265.1610592703732.JavaMail.xmail@bj-wm-cp-9>
2021-01-14  3:21     ` Steve Magnani
2021-01-13 22:25       ` changlianzhi
2021-01-14  7:57 lianzhi chang
2021-01-14 10:41 ` Jan Kara
2021-01-14 11:06 ` kernel test robot
2021-01-14 13:26 lianzhi chang
2021-01-14 17:15 ` Jan Kara

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).