[PATCH] jfs: fix potential null ptr deref in dbUpdatePMap

[Haoyang Liu]

Fix a potential null pointer dereference bug, check if the variable mp
is NULL before accessing mp->data, if mp is NULL, then return -EIO.

Signed-off-by: Haoyang Liu <ttttur...@hust.edu.cn>
---
fs/jfs/jfs_dmap.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
index f9009e4f9ffd..eee521a56280 100644
--- a/fs/jfs/jfs_dmap.c
+++ b/fs/jfs/jfs_dmap.c
@@ -493,6 +493,10 @@ dbUpdatePMap(struct inode *ipbmap,
return -EIO;
metapage_wait_for_io(mp);
}
+
+ if (mp == NULL)
+ return -EIO;
+
dp = (struct dmap *) mp->data;

/* determine the bit number and word within the dmap of
--
2.48.1

[Dan Carpenter]

I think the original code is okay as-is.

fs/jfs/jfs_dmap.c
477 /*
478 * update the block state a dmap at a time.
479 */
480 mp = NULL;
481 lastlblkno = 0;
^^^^^^^^^^^^^^^
the previous block is zero.

482 for (rem = nblocks; rem > 0; rem -= nblks, blkno += nblks) {
483 /* get the buffer for the current dmap. */
484 lblkno = BLKTODMAP(blkno, bmp->db_l2nbperpage);
485 if (lblkno != lastlblkno) {

if the current block is not the same as the previous block then true

486 if (mp) {

this is testing if we are on the first iteration

487 write_metapage(mp);
488 }
489
490 mp = read_metapage(bmp->db_ipbmap, lblkno, PSIZE,
491 0);
492 if (mp == NULL)
493 return -EIO;

So after the first iteration then mp can't be NULL.

494 metapage_wait_for_io(mp);
495 }
496
497 if (mp == NULL)
498 return -EIO;
499
500 dp = (struct dmap *) mp->data;

The only way this could trigger is if "lblkno" was zero and I think in
that case we are already doomed.

On Tue, Feb 11, 2025 at 04:09:45AM +0800, Haoyang Liu wrote:
> Fix a potential null pointer dereference bug, check if the variable mp
> is NULL before accessing mp->data, if mp is NULL, then return -EIO.
>
> Signed-off-by: Haoyang Liu <ttttur...@hust.edu.cn>

No Fixes tag.

> ---
> fs/jfs/jfs_dmap.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index f9009e4f9ffd..eee521a56280 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -493,6 +493,10 @@ dbUpdatePMap(struct inode *ipbmap,
> return -EIO;
> metapage_wait_for_io(mp);
> }
> +
> + if (mp == NULL)

Run checkpatch on your patches. This should have been if (!mp).
Anyway, this patch is not required so don't resend but next time run
checkpatch. ;)

regards,
dan carpenter

[Haoyang Liu]

Hi Dan,

Thanks for your explanation.

>
> On Tue, Feb 11, 2025 at 04:09:45AM +0800, Haoyang Liu wrote:
>> Fix a potential null pointer dereference bug, check if the variable mp
>> is NULL before accessing mp->data, if mp is NULL, then return -EIO.
>>
>> Signed-off-by: Haoyang Liu <ttttur...@hust.edu.cn>
> No Fixes tag.
>
>> ---
>> fs/jfs/jfs_dmap.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
>> index f9009e4f9ffd..eee521a56280 100644
>> --- a/fs/jfs/jfs_dmap.c
>> +++ b/fs/jfs/jfs_dmap.c
>> @@ -493,6 +493,10 @@ dbUpdatePMap(struct inode *ipbmap,
>> return -EIO;
>> metapage_wait_for_io(mp);
>> }
>> +
>> + if (mp == NULL)
> Run checkpatch on your patches. This should have been if (!mp).
> Anyway, this patch is not required so don't resend but next time run
> checkpatch. ;)

I did run checkpatch on this patch, but it didn't show any warnings or
errors. And I will always keep in mind to run checkpatch before sending.

Thanks,

Haoyang

[Dan Carpenter]

On Wed, Feb 12, 2025 at 11:58:34PM +0800, Haoyang Liu wrote:
> > > diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> > > index f9009e4f9ffd..eee521a56280 100644
> > > --- a/fs/jfs/jfs_dmap.c
> > > +++ b/fs/jfs/jfs_dmap.c
> > > @@ -493,6 +493,10 @@ dbUpdatePMap(struct inode *ipbmap,
> > > return -EIO;
> > > metapage_wait_for_io(mp);
> > > }
> > > +
> > > + if (mp == NULL)
> > Run checkpatch on your patches. This should have been if (!mp).
> > Anyway, this patch is not required so don't resend but next time run
> > checkpatch. ;)
>
> I did run checkpatch on this patch, but it didn't show any warnings or
> errors. And I will always keep in mind to run checkpatch before sending.

Ah. Yes. You're right. You need to add the --strict option to see this
warning. Always use --strict.

regards,
dan carpenter