Patchwork [-next] ext4: locking issue on error path

login
register
mail settings
Submitter Dan Carpenter
Date July 17, 2012, 6:31 a.m.
Message ID <20120717063106.GA14985@elgon.mountain>
Download mbox | patch
Permalink /patch/171343/
State Accepted
Headers show

Comments

Dan Carpenter - July 17, 2012, 6:31 a.m.
We recently changed how the locking worked here, but this error path was
missed.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Zheng Liu - July 17, 2012, 8:13 a.m.
On Tue, Jul 17, 2012 at 09:31:06AM +0300, Dan Carpenter wrote:
> We recently changed how the locking worked here, but this error path was
> missed.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Sorry, it is my fault.  Thanks for pointing out this bug.

Regards,
Zheng

> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 8c84070..2728fb7 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -3031,8 +3031,10 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
>  		if (!is_sync_kiocb(iocb)) {
>  			ext4_io_end_t *io_end =
>  				ext4_init_io_end(inode, GFP_NOFS);
> -			if (!io_end)
> -				return -ENOMEM;
> +			if (!io_end) {
> +				ret = -ENOMEM;
> +				goto retake_lock;
> +			}
>  			io_end->flag |= EXT4_IO_END_DIRECT;
>  			iocb->private = io_end;
>  			/*
> @@ -3094,6 +3096,7 @@ static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
>  			ext4_clear_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
>  		}
>  
> +retake_lock:
>  		/* take i_mutex locking again if we do a ovewrite dio */
>  		if (overwrite) {
>  			up_read(&EXT4_I(inode)->i_data_sem);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Theodore Ts'o - July 23, 2012, 12:20 a.m.
On Tue, Jul 17, 2012 at 04:13:51PM +0800, Zheng Liu wrote:
> On Tue, Jul 17, 2012 at 09:31:06AM +0300, Dan Carpenter wrote:
> > We recently changed how the locking worked here, but this error path was
> > missed.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> Sorry, it is my fault.  Thanks for pointing out this bug.

Since this patch hadn't been promoted past the master branch pointer
(it was only in the dev branch, which can be rebased), I've merged
this fix up with Zheng's original patch.

Dan, may thanks for finding and pointing it out!!

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 8c84070..2728fb7 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3031,8 +3031,10 @@  static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
 		if (!is_sync_kiocb(iocb)) {
 			ext4_io_end_t *io_end =
 				ext4_init_io_end(inode, GFP_NOFS);
-			if (!io_end)
-				return -ENOMEM;
+			if (!io_end) {
+				ret = -ENOMEM;
+				goto retake_lock;
+			}
 			io_end->flag |= EXT4_IO_END_DIRECT;
 			iocb->private = io_end;
 			/*
@@ -3094,6 +3096,7 @@  static ssize_t ext4_ext_direct_IO(int rw, struct kiocb *iocb,
 			ext4_clear_inode_state(inode, EXT4_STATE_DIO_UNWRITTEN);
 		}
 
+retake_lock:
 		/* take i_mutex locking again if we do a ovewrite dio */
 		if (overwrite) {
 			up_read(&EXT4_I(inode)->i_data_sem);