Re: [PATCH 1/2] xfs: Check the return value of xfs_buf_read() for NULL

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2011-08-02 at 19:18 -0700, Chandra Seetharaman wrote:
> Check the return value of xfs_buf_read() for NULL and return ENOMEM if it is NULL.

You might add that the reason for doing this is that in
these spots the null buffer pointer would be blindly
dereferenced otherwise.

One piece of this patch needs to go, so please post an
updated version.  But you can consider this reviewed by me.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

> Signed-off-by: Chandra Seetharaman <sekharan@xxxxxxxxxx>
> ---
>  fs/xfs/xfs_log_recover.c |    6 ++++++
>  fs/xfs/xfs_rw.c          |    2 +-
>  fs/xfs/xfs_vnodeops.c    |    2 ++
>  3 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 93786e5..8478532 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -2131,6 +2131,8 @@ xlog_recover_buffer_pass2(
>  
>  	bp = xfs_buf_read(mp->m_ddev_targp, buf_f->blf_blkno, buf_f->blf_len,
>  			  buf_flags);
> +	if (!bp)
> +		return XFS_ERROR(ENOMEM);
>  	error = xfs_buf_geterror(bp);
>  	if (error) {
>  		xfs_ioerror_alert("xlog_recover_do..(read#1)", mp,
> @@ -2222,6 +2224,10 @@ xlog_recover_inode_pass2(
>  
>  	bp = xfs_buf_read(mp->m_ddev_targp, in_f->ilf_blkno, in_f->ilf_len,
>  			  XBF_LOCK);
> +	if (!bp) {
> +		error = ENOMEM;
> +		goto error;
> +	}
>  	error = xfs_buf_geterror(bp);
>  	if (error) {
>  		xfs_ioerror_alert("xlog_recover_do..(read#2)", mp,
> diff --git a/fs/xfs/xfs_rw.c b/fs/xfs/xfs_rw.c
> index c96a8a0..ecaa60e 100644
> --- a/fs/xfs/xfs_rw.c
> +++ b/fs/xfs/xfs_rw.c
> @@ -136,7 +136,7 @@ xfs_read_buf(
>  
>  	bp = xfs_buf_read(target, blkno, len, flags);
>  	if (!bp)
> -		return XFS_ERROR(EIO);
> +		return (flags & XBF_TRYLOCK) ?  0 : XFS_ERROR(EIO);

This change doesn't belong in this patch.

As far as I can tell, no call to xfs_read_buf()
ever passes XBF_TRYLOCK in flags anyway (only
XBF_LOCK, XBF_DONT_BLOCK, and XBF_MAPPED).

>  	error = bp->b_error;
>  	if (!error && !XFS_FORCED_SHUTDOWN(mp)) {
>  		*bpp = bp;
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 3ee5f8a..64a5835 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -83,6 +83,8 @@ xfs_readlink_bmap(
>  
>  		bp = xfs_buf_read(mp->m_ddev_targp, d, BTOBB(byte_cnt),
>  				  XBF_LOCK | XBF_MAPPED | XBF_DONT_BLOCK);
> +		if (!bp)
> +			return XFS_ERROR(ENOMEM);
>  		error = xfs_buf_geterror(bp);
>  		if (error) {
>  			xfs_ioerror_alert("xfs_readlink",



_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs


[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux