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

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

[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

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

[Jan Kara]

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

Re: [PATCH] udf: fix the problem that the disc content is not displayed【Suspected phishing email, please pay attention to password security】

[changlianzhi]

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

>--
>Jan Kara <jack@suse.com>
>SUSE Labs, CR

sorry,I will resubmit it again！thank you very much!

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

[kernel test robot]

[-- 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 --]

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

[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

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

[Jan Kara]

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

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

[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

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

[Steve Magnani]

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>

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

[Steve Magnani]

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>

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

[changlianzhi]

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