Re: [PATCH] ufs: Handle NULL return from ufs_get_locked_folio()

[Jan Kara]

On Fri 15-05-26 04:32:56, Ingyu Jang wrote:
> ufs_get_locked_folio() may return either an error pointer (when
> read_mapping_folio() fails) or NULL (when a concurrent truncate has
> detached the folio from its mapping). The current IS_ERR() check in
> ufs_alloc_lastblock() only handles the error-pointer case; a NULL
> return falls through to folio_buffers(folio), causing a NULL pointer
> dereference under the truncate race.
> 
> Use IS_ERR_OR_NULL() to cover both failure modes.
> 
> Signed-off-by: Ingyu Jang <ingyujang25@xxxxxxxxxxx>

Well, but this shouldn't really happen because ufs_alloc_lastblock() is
called only from ufs_truncate() where it is protected from folio being
detached by i_rwsem. Still probably makes sense from code robustness POV
though so feel free to add:

Reviewed-by: Jan Kara <jack@xxxxxxx>

but please update changelog to reflect this.

								Honza

> ---
>  fs/ufs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/ufs/inode.c b/fs/ufs/inode.c
> index 440d014cc5ed5..989ff3d3ad179 100644
> --- a/fs/ufs/inode.c
> +++ b/fs/ufs/inode.c
> @@ -1051,7 +1051,7 @@ static int ufs_alloc_lastblock(struct inode *inode, loff_t size)
>  
>  	folio = ufs_get_locked_folio(mapping, lastfrag >>
>  				       (PAGE_SHIFT - inode->i_blkbits));
> -	if (IS_ERR(folio)) {
> +	if (IS_ERR_OR_NULL(folio)) {
>  		err = -EIO;
>  		goto out;
>  	}
> -- 
> 2.34.1
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR