Messages in this thread | | | Date | Wed, 22 Dec 2021 01:38:28 +0000 | Subject | Re: [PATCH] fs/squashfs: handle possible null pointer | From | Phillip Lougher <> |
| |
On 21/12/2021 02:03, Peng Hao wrote: > in squashfs_fill_super: > > msblk->decompressor = supported_squashfs_filesystem( > fc, > le16_to_cpu(sblk->s_major), > le16_to_cpu(sblk->s_minor), > le16_to_cpu(sblk->compression)); > if (msblk->decompressor == NULL) > goto failed_mount; > ... > > failed_mount: > ... > squashfs_decompressor_destroy(msblk); > > in squashfs_decompressor_destroy: > if (stream) { > msblk->decompressor->free(stream->stream); > msblk->decompressor is NULL. > > so add a judgment whether a null pointer.
NACK.
The NULL pointer dereference (msblk->decompressor) will not happen because stream (msblk->stream) is NULL (thus the code in question will not be executed).
At the start of the initialisation phase msblk is zeroed out
sb->s_fs_info = kzalloc(sizeof(*msblk), GFP_KERNEL); if (sb->s_fs_info == NULL) { ERROR("Failed to allocate squashfs_sb_info\n"); return -ENOMEM; } msblk = sb->s_fs_info;
Making msblk->decompressor and msblk->stream NULL.
msblk->decompressor is allocated first
msblk->decompressor = supported_squashfs_filesystem( fc, le16_to_cpu(sblk->s_major), le16_to_cpu(sblk->s_minor), le16_to_cpu(sblk->compression)); if (msblk->decompressor == NULL) goto failed_mount;
If that succeeds (msblk->decompressor != NULL), then msblk->stream is allocated
msblk->stream = squashfs_decompressor_setup(sb, flags); if (IS_ERR(msblk->stream)) { err = PTR_ERR(msblk->stream); msblk->stream = NULL; goto insanity; }
Thus, in squashfs_decompressor_destroy() if stream (msblk->stream) is not NULL, then msblk->decompressor is also guaranteed to be not NULL.
Likewise if msblk->decompressor is NULL, then msblk->stream will also be NULL.
Or to put it another way, the decompressor is only destroyed if it was previously created (msblk->stream != NULL), and it will only have been created if msblk->decompressor is != NULL.
Phillip
| |