linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] fs: affs: fix a NULL pointer dereference
@ 2019-03-14  7:46 Kangjie Lu
  2019-03-14 14:51 ` kbuild test robot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Kangjie Lu @ 2019-03-14  7:46 UTC (permalink / raw)
  To: kjlu; +Cc: pakki001, David Sterba, linux-fsdevel, linux-kernel

If affs_bread fails, do not use ext_bh to avoid NULL pointer
 dereference

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 fs/affs/file.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/affs/file.c b/fs/affs/file.c
index a85817f54483..45b96faa40f1 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -835,7 +835,7 @@ void
 affs_truncate(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
-	u32 ext, ext_key;
+	u32 ext, ext_key, ext_bk;
 	u32 last_blk, blkcnt, blk;
 	u32 size;
 	struct buffer_head *ext_bh;
@@ -941,8 +941,12 @@ affs_truncate(struct inode *inode)
 		size = AFFS_SB(sb)->s_hashsize;
 		if (size > blkcnt - blk)
 			size = blkcnt - blk;
-		for (i = 0; i < size; i++, blk++)
-			affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
+		if (ext_bh) {
+			for (i = 0; i < size; i++, blk++) {
+				ext_bk = AFFS_BLOCK(sb, ext_bh, i);
+				affs_free_block(sb, be32_to_cpu(ext_bk));
+			}
+		}
 		affs_free_block(sb, ext_key);
 		ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
 		affs_brelse(ext_bh);
-- 
2.17.1


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

* Re: [PATCH] fs: affs: fix a NULL pointer dereference
  2019-03-14  7:46 [PATCH] fs: affs: fix a NULL pointer dereference Kangjie Lu
@ 2019-03-14 14:51 ` kbuild test robot
  2019-03-19  9:00 ` Dan Carpenter
  2019-03-19 13:43 ` Geert Uytterhoeven
  2 siblings, 0 replies; 6+ messages in thread
From: kbuild test robot @ 2019-03-14 14:51 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: kbuild-all, kjlu, pakki001, David Sterba, linux-fsdevel, linux-kernel

Hi Kangjie,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.0 next-20190306]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Kangjie-Lu/fs-affs-fix-a-NULL-pointer-dereference/20190314-170334
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'


sparse warnings: (new ones prefixed by >>)

   fs/affs/file.c:525:23: sparse: expression using sizeof(void)
   fs/affs/file.c:525:23: sparse: expression using sizeof(void)
   fs/affs/file.c:558:23: sparse: expression using sizeof(void)
   fs/affs/file.c:558:23: sparse: expression using sizeof(void)
   fs/affs/file.c:577:23: sparse: expression using sizeof(void)
   fs/affs/file.c:577:23: sparse: expression using sizeof(void)
   fs/affs/file.c:706:23: sparse: expression using sizeof(void)
   fs/affs/file.c:706:23: sparse: expression using sizeof(void)
   fs/affs/file.c:759:23: sparse: expression using sizeof(void)
   fs/affs/file.c:759:23: sparse: expression using sizeof(void)
>> fs/affs/file.c:946:40: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [unsigned] [usertype] ext_bk @@    got igned] [usertype] ext_bk @@
   fs/affs/file.c:946:40:    expected unsigned int [unsigned] [usertype] ext_bk
   fs/affs/file.c:946:40:    got restricted __be32 <noident>
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32
>> fs/affs/file.c:947:53: sparse: cast to restricted __be32

vim +946 fs/affs/file.c

   833	
   834	void
   835	affs_truncate(struct inode *inode)
   836	{
   837		struct super_block *sb = inode->i_sb;
   838		u32 ext, ext_key, ext_bk;
   839		u32 last_blk, blkcnt, blk;
   840		u32 size;
   841		struct buffer_head *ext_bh;
   842		int i;
   843	
   844		pr_debug("truncate(inode=%lu, oldsize=%llu, newsize=%llu)\n",
   845			 inode->i_ino, AFFS_I(inode)->mmu_private, inode->i_size);
   846	
   847		last_blk = 0;
   848		ext = 0;
   849		if (inode->i_size) {
   850			last_blk = ((u32)inode->i_size - 1) / AFFS_SB(sb)->s_data_blksize;
   851			ext = last_blk / AFFS_SB(sb)->s_hashsize;
   852		}
   853	
   854		if (inode->i_size > AFFS_I(inode)->mmu_private) {
   855			struct address_space *mapping = inode->i_mapping;
   856			struct page *page;
   857			void *fsdata;
   858			loff_t isize = inode->i_size;
   859			int res;
   860	
   861			res = mapping->a_ops->write_begin(NULL, mapping, isize, 0, 0, &page, &fsdata);
   862			if (!res)
   863				res = mapping->a_ops->write_end(NULL, mapping, isize, 0, 0, page, fsdata);
   864			else
   865				inode->i_size = AFFS_I(inode)->mmu_private;
   866			mark_inode_dirty(inode);
   867			return;
   868		} else if (inode->i_size == AFFS_I(inode)->mmu_private)
   869			return;
   870	
   871		// lock cache
   872		ext_bh = affs_get_extblock(inode, ext);
   873		if (IS_ERR(ext_bh)) {
   874			affs_warning(sb, "truncate",
   875				     "unexpected read error for ext block %u (%ld)",
   876				     ext, PTR_ERR(ext_bh));
   877			return;
   878		}
   879		if (AFFS_I(inode)->i_lc) {
   880			/* clear linear cache */
   881			i = (ext + 1) >> AFFS_I(inode)->i_lc_shift;
   882			if (AFFS_I(inode)->i_lc_size > i) {
   883				AFFS_I(inode)->i_lc_size = i;
   884				for (; i < AFFS_LC_SIZE; i++)
   885					AFFS_I(inode)->i_lc[i] = 0;
   886			}
   887			/* clear associative cache */
   888			for (i = 0; i < AFFS_AC_SIZE; i++)
   889				if (AFFS_I(inode)->i_ac[i].ext >= ext)
   890					AFFS_I(inode)->i_ac[i].ext = 0;
   891		}
   892		ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
   893	
   894		blkcnt = AFFS_I(inode)->i_blkcnt;
   895		i = 0;
   896		blk = last_blk;
   897		if (inode->i_size) {
   898			i = last_blk % AFFS_SB(sb)->s_hashsize + 1;
   899			blk++;
   900		} else
   901			AFFS_HEAD(ext_bh)->first_data = 0;
   902		AFFS_HEAD(ext_bh)->block_count = cpu_to_be32(i);
   903		size = AFFS_SB(sb)->s_hashsize;
   904		if (size > blkcnt - blk + i)
   905			size = blkcnt - blk + i;
   906		for (; i < size; i++, blk++) {
   907			affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
   908			AFFS_BLOCK(sb, ext_bh, i) = 0;
   909		}
   910		AFFS_TAIL(sb, ext_bh)->extension = 0;
   911		affs_fix_checksum(sb, ext_bh);
   912		mark_buffer_dirty_inode(ext_bh, inode);
   913		affs_brelse(ext_bh);
   914	
   915		if (inode->i_size) {
   916			AFFS_I(inode)->i_blkcnt = last_blk + 1;
   917			AFFS_I(inode)->i_extcnt = ext + 1;
   918			if (affs_test_opt(AFFS_SB(sb)->s_flags, SF_OFS)) {
   919				struct buffer_head *bh = affs_bread_ino(inode, last_blk, 0);
   920				u32 tmp;
   921				if (IS_ERR(bh)) {
   922					affs_warning(sb, "truncate",
   923						     "unexpected read error for last block %u (%ld)",
   924						     ext, PTR_ERR(bh));
   925					return;
   926				}
   927				tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);
   928				AFFS_DATA_HEAD(bh)->next = 0;
   929				affs_adjust_checksum(bh, -tmp);
   930				affs_brelse(bh);
   931			}
   932		} else {
   933			AFFS_I(inode)->i_blkcnt = 0;
   934			AFFS_I(inode)->i_extcnt = 1;
   935		}
   936		AFFS_I(inode)->mmu_private = inode->i_size;
   937		// unlock cache
   938	
   939		while (ext_key) {
   940			ext_bh = affs_bread(sb, ext_key);
   941			size = AFFS_SB(sb)->s_hashsize;
   942			if (size > blkcnt - blk)
   943				size = blkcnt - blk;
   944			if (ext_bh) {
   945				for (i = 0; i < size; i++, blk++) {
 > 946					ext_bk = AFFS_BLOCK(sb, ext_bh, i);
 > 947					affs_free_block(sb, be32_to_cpu(ext_bk));
   948				}
   949			}
   950			affs_free_block(sb, ext_key);
   951			ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
   952			affs_brelse(ext_bh);
   953		}
   954		affs_free_prealloc(inode);
   955	}
   956	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH] fs: affs: fix a NULL pointer dereference
  2019-03-14  7:46 [PATCH] fs: affs: fix a NULL pointer dereference Kangjie Lu
  2019-03-14 14:51 ` kbuild test robot
@ 2019-03-19  9:00 ` Dan Carpenter
  2019-03-19 13:43 ` Geert Uytterhoeven
  2 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2019-03-19  9:00 UTC (permalink / raw)
  To: kbuild, Kangjie Lu
  Cc: kbuild-all, kjlu, pakki001, David Sterba, linux-fsdevel, linux-kernel

Hi Kangjie,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Kangjie-Lu/fs-affs-fix-a-NULL-pointer-dereference/20190314-170334

New smatch warnings:
fs/affs/file.c:951 affs_truncate() error: we previously assumed 'ext_bh' could be null (see line 944)

Old smatch warnings:
fs/affs/file.c:806 affs_write_end_ofs() warn: passing zero to 'PTR_ERR'

# https://github.com/0day-ci/linux/commit/2ee20c56bd586ddaf3ebdb1c3cad26439edc9eb6
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 2ee20c56bd586ddaf3ebdb1c3cad26439edc9eb6
vim +/ext_bh +951 fs/affs/file.c

^1da177e4 Linus Torvalds     2005-04-16  833  
^1da177e4 Linus Torvalds     2005-04-16  834  void
^1da177e4 Linus Torvalds     2005-04-16  835  affs_truncate(struct inode *inode)
^1da177e4 Linus Torvalds     2005-04-16  836  {
^1da177e4 Linus Torvalds     2005-04-16  837  	struct super_block *sb = inode->i_sb;
2ee20c56b Kangjie Lu         2019-03-14  838  	u32 ext, ext_key, ext_bk;
^1da177e4 Linus Torvalds     2005-04-16  839  	u32 last_blk, blkcnt, blk;
^1da177e4 Linus Torvalds     2005-04-16  840  	u32 size;
^1da177e4 Linus Torvalds     2005-04-16  841  	struct buffer_head *ext_bh;
^1da177e4 Linus Torvalds     2005-04-16  842  	int i;
^1da177e4 Linus Torvalds     2005-04-16  843  
08fe100d9 Geert Uytterhoeven 2015-02-17  844  	pr_debug("truncate(inode=%lu, oldsize=%llu, newsize=%llu)\n",
08fe100d9 Geert Uytterhoeven 2015-02-17  845  		 inode->i_ino, AFFS_I(inode)->mmu_private, inode->i_size);
^1da177e4 Linus Torvalds     2005-04-16  846  
^1da177e4 Linus Torvalds     2005-04-16  847  	last_blk = 0;
^1da177e4 Linus Torvalds     2005-04-16  848  	ext = 0;
^1da177e4 Linus Torvalds     2005-04-16  849  	if (inode->i_size) {
^1da177e4 Linus Torvalds     2005-04-16  850  		last_blk = ((u32)inode->i_size - 1) / AFFS_SB(sb)->s_data_blksize;
^1da177e4 Linus Torvalds     2005-04-16  851  		ext = last_blk / AFFS_SB(sb)->s_hashsize;
^1da177e4 Linus Torvalds     2005-04-16  852  	}
^1da177e4 Linus Torvalds     2005-04-16  853  
^1da177e4 Linus Torvalds     2005-04-16  854  	if (inode->i_size > AFFS_I(inode)->mmu_private) {
^1da177e4 Linus Torvalds     2005-04-16  855  		struct address_space *mapping = inode->i_mapping;
^1da177e4 Linus Torvalds     2005-04-16  856  		struct page *page;
f2b6a16eb Nick Piggin        2007-10-16  857  		void *fsdata;
73516ace9 Fabian Frederick   2014-10-13  858  		loff_t isize = inode->i_size;
^1da177e4 Linus Torvalds     2005-04-16  859  		int res;
^1da177e4 Linus Torvalds     2005-04-16  860  
73516ace9 Fabian Frederick   2014-10-13  861  		res = mapping->a_ops->write_begin(NULL, mapping, isize, 0, 0, &page, &fsdata);
^1da177e4 Linus Torvalds     2005-04-16  862  		if (!res)
73516ace9 Fabian Frederick   2014-10-13  863  			res = mapping->a_ops->write_end(NULL, mapping, isize, 0, 0, page, fsdata);
dca3c3365 Roman Zippel       2008-04-29  864  		else
dca3c3365 Roman Zippel       2008-04-29  865  			inode->i_size = AFFS_I(inode)->mmu_private;
^1da177e4 Linus Torvalds     2005-04-16  866  		mark_inode_dirty(inode);
^1da177e4 Linus Torvalds     2005-04-16  867  		return;
^1da177e4 Linus Torvalds     2005-04-16  868  	} else if (inode->i_size == AFFS_I(inode)->mmu_private)
^1da177e4 Linus Torvalds     2005-04-16  869  		return;
^1da177e4 Linus Torvalds     2005-04-16  870  
^1da177e4 Linus Torvalds     2005-04-16  871  	// lock cache
^1da177e4 Linus Torvalds     2005-04-16  872  	ext_bh = affs_get_extblock(inode, ext);
^1da177e4 Linus Torvalds     2005-04-16  873  	if (IS_ERR(ext_bh)) {
1ee54b099 Fabian Frederick   2014-12-12  874  		affs_warning(sb, "truncate",
1ee54b099 Fabian Frederick   2014-12-12  875  			     "unexpected read error for ext block %u (%ld)",
08fe100d9 Geert Uytterhoeven 2015-02-17  876  			     ext, PTR_ERR(ext_bh));
^1da177e4 Linus Torvalds     2005-04-16  877  		return;
^1da177e4 Linus Torvalds     2005-04-16  878  	}
^1da177e4 Linus Torvalds     2005-04-16  879  	if (AFFS_I(inode)->i_lc) {
^1da177e4 Linus Torvalds     2005-04-16  880  		/* clear linear cache */
^1da177e4 Linus Torvalds     2005-04-16  881  		i = (ext + 1) >> AFFS_I(inode)->i_lc_shift;
^1da177e4 Linus Torvalds     2005-04-16  882  		if (AFFS_I(inode)->i_lc_size > i) {
^1da177e4 Linus Torvalds     2005-04-16  883  			AFFS_I(inode)->i_lc_size = i;
^1da177e4 Linus Torvalds     2005-04-16  884  			for (; i < AFFS_LC_SIZE; i++)
^1da177e4 Linus Torvalds     2005-04-16  885  				AFFS_I(inode)->i_lc[i] = 0;
^1da177e4 Linus Torvalds     2005-04-16  886  		}
^1da177e4 Linus Torvalds     2005-04-16  887  		/* clear associative cache */
^1da177e4 Linus Torvalds     2005-04-16  888  		for (i = 0; i < AFFS_AC_SIZE; i++)
^1da177e4 Linus Torvalds     2005-04-16  889  			if (AFFS_I(inode)->i_ac[i].ext >= ext)
^1da177e4 Linus Torvalds     2005-04-16  890  				AFFS_I(inode)->i_ac[i].ext = 0;
^1da177e4 Linus Torvalds     2005-04-16  891  	}
^1da177e4 Linus Torvalds     2005-04-16  892  	ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
^1da177e4 Linus Torvalds     2005-04-16  893  
^1da177e4 Linus Torvalds     2005-04-16  894  	blkcnt = AFFS_I(inode)->i_blkcnt;
^1da177e4 Linus Torvalds     2005-04-16  895  	i = 0;
^1da177e4 Linus Torvalds     2005-04-16  896  	blk = last_blk;
^1da177e4 Linus Torvalds     2005-04-16  897  	if (inode->i_size) {
^1da177e4 Linus Torvalds     2005-04-16  898  		i = last_blk % AFFS_SB(sb)->s_hashsize + 1;
^1da177e4 Linus Torvalds     2005-04-16  899  		blk++;
^1da177e4 Linus Torvalds     2005-04-16  900  	} else
^1da177e4 Linus Torvalds     2005-04-16  901  		AFFS_HEAD(ext_bh)->first_data = 0;
dca3c3365 Roman Zippel       2008-04-29  902  	AFFS_HEAD(ext_bh)->block_count = cpu_to_be32(i);
^1da177e4 Linus Torvalds     2005-04-16  903  	size = AFFS_SB(sb)->s_hashsize;
^1da177e4 Linus Torvalds     2005-04-16  904  	if (size > blkcnt - blk + i)
^1da177e4 Linus Torvalds     2005-04-16  905  		size = blkcnt - blk + i;
^1da177e4 Linus Torvalds     2005-04-16  906  	for (; i < size; i++, blk++) {
^1da177e4 Linus Torvalds     2005-04-16  907  		affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
^1da177e4 Linus Torvalds     2005-04-16  908  		AFFS_BLOCK(sb, ext_bh, i) = 0;
^1da177e4 Linus Torvalds     2005-04-16  909  	}
^1da177e4 Linus Torvalds     2005-04-16  910  	AFFS_TAIL(sb, ext_bh)->extension = 0;
^1da177e4 Linus Torvalds     2005-04-16  911  	affs_fix_checksum(sb, ext_bh);
^1da177e4 Linus Torvalds     2005-04-16  912  	mark_buffer_dirty_inode(ext_bh, inode);
^1da177e4 Linus Torvalds     2005-04-16  913  	affs_brelse(ext_bh);
^1da177e4 Linus Torvalds     2005-04-16  914  
^1da177e4 Linus Torvalds     2005-04-16  915  	if (inode->i_size) {
^1da177e4 Linus Torvalds     2005-04-16  916  		AFFS_I(inode)->i_blkcnt = last_blk + 1;
^1da177e4 Linus Torvalds     2005-04-16  917  		AFFS_I(inode)->i_extcnt = ext + 1;
79bda4d51 Fabian Frederick   2015-04-16  918  		if (affs_test_opt(AFFS_SB(sb)->s_flags, SF_OFS)) {
^1da177e4 Linus Torvalds     2005-04-16  919  			struct buffer_head *bh = affs_bread_ino(inode, last_blk, 0);
^1da177e4 Linus Torvalds     2005-04-16  920  			u32 tmp;
0e45b67d5 Dan Carpenter      2010-08-25  921  			if (IS_ERR(bh)) {
1ee54b099 Fabian Frederick   2014-12-12  922  				affs_warning(sb, "truncate",
1ee54b099 Fabian Frederick   2014-12-12  923  					     "unexpected read error for last block %u (%ld)",
08fe100d9 Geert Uytterhoeven 2015-02-17  924  					     ext, PTR_ERR(bh));
^1da177e4 Linus Torvalds     2005-04-16  925  				return;
^1da177e4 Linus Torvalds     2005-04-16  926  			}
^1da177e4 Linus Torvalds     2005-04-16  927  			tmp = be32_to_cpu(AFFS_DATA_HEAD(bh)->next);
^1da177e4 Linus Torvalds     2005-04-16  928  			AFFS_DATA_HEAD(bh)->next = 0;
^1da177e4 Linus Torvalds     2005-04-16  929  			affs_adjust_checksum(bh, -tmp);
^1da177e4 Linus Torvalds     2005-04-16  930  			affs_brelse(bh);
^1da177e4 Linus Torvalds     2005-04-16  931  		}
^1da177e4 Linus Torvalds     2005-04-16  932  	} else {
^1da177e4 Linus Torvalds     2005-04-16  933  		AFFS_I(inode)->i_blkcnt = 0;
^1da177e4 Linus Torvalds     2005-04-16  934  		AFFS_I(inode)->i_extcnt = 1;
^1da177e4 Linus Torvalds     2005-04-16  935  	}
^1da177e4 Linus Torvalds     2005-04-16  936  	AFFS_I(inode)->mmu_private = inode->i_size;
^1da177e4 Linus Torvalds     2005-04-16  937  	// unlock cache
^1da177e4 Linus Torvalds     2005-04-16  938  
^1da177e4 Linus Torvalds     2005-04-16  939  	while (ext_key) {
^1da177e4 Linus Torvalds     2005-04-16  940  		ext_bh = affs_bread(sb, ext_key);
^1da177e4 Linus Torvalds     2005-04-16  941  		size = AFFS_SB(sb)->s_hashsize;
^1da177e4 Linus Torvalds     2005-04-16  942  		if (size > blkcnt - blk)
^1da177e4 Linus Torvalds     2005-04-16  943  			size = blkcnt - blk;
2ee20c56b Kangjie Lu         2019-03-14 @944  		if (ext_bh) {
2ee20c56b Kangjie Lu         2019-03-14  945  			for (i = 0; i < size; i++, blk++) {
2ee20c56b Kangjie Lu         2019-03-14  946  				ext_bk = AFFS_BLOCK(sb, ext_bh, i);
2ee20c56b Kangjie Lu         2019-03-14  947  				affs_free_block(sb, be32_to_cpu(ext_bk));
2ee20c56b Kangjie Lu         2019-03-14  948  			}
2ee20c56b Kangjie Lu         2019-03-14  949  		}
^1da177e4 Linus Torvalds     2005-04-16  950  		affs_free_block(sb, ext_key);
^1da177e4 Linus Torvalds     2005-04-16 @951  		ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
^1da177e4 Linus Torvalds     2005-04-16  952  		affs_brelse(ext_bh);
^1da177e4 Linus Torvalds     2005-04-16  953  	}
^1da177e4 Linus Torvalds     2005-04-16  954  	affs_free_prealloc(inode);
^1da177e4 Linus Torvalds     2005-04-16  955  }
c47587955 Al Viro            2009-06-08  956  

:::::: The code at line 951 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2

:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

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

* Re: [PATCH] fs: affs: fix a NULL pointer dereference
  2019-03-14  7:46 [PATCH] fs: affs: fix a NULL pointer dereference Kangjie Lu
  2019-03-14 14:51 ` kbuild test robot
  2019-03-19  9:00 ` Dan Carpenter
@ 2019-03-19 13:43 ` Geert Uytterhoeven
  2 siblings, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2019-03-19 13:43 UTC (permalink / raw)
  To: Kangjie Lu
  Cc: pakki001, David Sterba, Linux FS Devel, Linux Kernel Mailing List

Hi Kangjie,

On Thu, Mar 14, 2019 at 8:47 AM Kangjie Lu <kjlu@umn.edu> wrote:
> If affs_bread fails, do not use ext_bh to avoid NULL pointer
>  dereference
>
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Thanks for your patch!

> --- a/fs/affs/file.c
> +++ b/fs/affs/file.c
> @@ -835,7 +835,7 @@ void
>  affs_truncate(struct inode *inode)
>  {
>         struct super_block *sb = inode->i_sb;
> -       u32 ext, ext_key;
> +       u32 ext, ext_key, ext_bk;

Why adding an intermediate variable (without __be32 tag)?

>         u32 last_blk, blkcnt, blk;
>         u32 size;
>         struct buffer_head *ext_bh;
> @@ -941,8 +941,12 @@ affs_truncate(struct inode *inode)
>                 size = AFFS_SB(sb)->s_hashsize;
>                 if (size > blkcnt - blk)
>                         size = blkcnt - blk;
> -               for (i = 0; i < size; i++, blk++)
> -                       affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
> +               if (ext_bh) {
> +                       for (i = 0; i < size; i++, blk++) {
> +                               ext_bk = AFFS_BLOCK(sb, ext_bh, i);
> +                               affs_free_block(sb, be32_to_cpu(ext_bk));
> +                       }

Now this ignores all errors, silently.
What about handling actual errors, and propagating them up?

> +               }
>                 affs_free_block(sb, ext_key);
>                 ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
>                 affs_brelse(ext_bh);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH] fs: affs: fix a NULL pointer dereference
  2019-03-15  7:42 Kangjie Lu
@ 2019-03-21 14:45 ` David Sterba
  0 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2019-03-21 14:45 UTC (permalink / raw)
  To: Kangjie Lu; +Cc: pakki001, linux-fsdevel, linux-kernel

On Fri, Mar 15, 2019 at 02:42:44AM -0500, Kangjie Lu wrote:
> If affs_bread fails, do not use ext_bh to avoid NULL pointer
>  dereference
> 
> Signed-off-by: Kangjie Lu <kjlu@umn.edu>

Tanks for the patch.  I'll need some more time to get familiar with the
AFFS code to review your patch.

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

* [PATCH] fs: affs: fix a NULL pointer dereference
@ 2019-03-15  7:42 Kangjie Lu
  2019-03-21 14:45 ` David Sterba
  0 siblings, 1 reply; 6+ messages in thread
From: Kangjie Lu @ 2019-03-15  7:42 UTC (permalink / raw)
  To: kjlu; +Cc: pakki001, David Sterba, linux-fsdevel, linux-kernel

If affs_bread fails, do not use ext_bh to avoid NULL pointer
 dereference

Signed-off-by: Kangjie Lu <kjlu@umn.edu>
---
 fs/affs/file.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/affs/file.c b/fs/affs/file.c
index a85817f54483..29cbc8eda085 100644
--- a/fs/affs/file.c
+++ b/fs/affs/file.c
@@ -941,8 +941,10 @@ affs_truncate(struct inode *inode)
 		size = AFFS_SB(sb)->s_hashsize;
 		if (size > blkcnt - blk)
 			size = blkcnt - blk;
-		for (i = 0; i < size; i++, blk++)
-			affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
+		if (ext_bh) {
+			for (i = 0; i < size; i++, blk++)
+				affs_free_block(sb, be32_to_cpu(AFFS_BLOCK(sb, ext_bh, i)));
+		}
 		affs_free_block(sb, ext_key);
 		ext_key = be32_to_cpu(AFFS_TAIL(sb, ext_bh)->extension);
 		affs_brelse(ext_bh);
-- 
2.17.1


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

end of thread, other threads:[~2019-03-21 14:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-14  7:46 [PATCH] fs: affs: fix a NULL pointer dereference Kangjie Lu
2019-03-14 14:51 ` kbuild test robot
2019-03-19  9:00 ` Dan Carpenter
2019-03-19 13:43 ` Geert Uytterhoeven
2019-03-15  7:42 Kangjie Lu
2019-03-21 14:45 ` David Sterba

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).