2007-10-26 23:41:54

by Mike Waychison

Subject: [patch 4/6][RFC] Attempt to plug race with truncate

Attempt to deal with races with truncate paths.

I'm not really sure on the locking here, but these seem to be taken by the truncate path. BKL is left as some filesystem may(?) still require it.

Signed-off-by: Mike Waychison <[email protected]>
fs/ioctl.c | 8 ++++++++
1 file changed, 8 insertions(+)

Index: linux-2.6.23/fs/ioctl.c
===================================================================
--- linux-2.6.23.orig/fs/ioctl.c 2007-10-26 15:27:29.000000000 -0700
+++ linux-2.6.23/fs/ioctl.c 2007-10-26 16:16:28.000000000 -0700
@@ -43,13 +43,21 @@ static long do_ioctl(struct file *filp,
static int do_fibmap(struct address_space *mapping, sector_t block,
sector_t *phys_block)
{
+ struct inode *inode = mapping->host;
+
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
if (!mapping->a_ops->bmap)
return -EINVAL;

lock_kernel();
+ /* Avoid races with truncate */
+ mutex_lock(&inode->i_mutex);
+ /* FIXME: Do we really need i_alloc_sem? */
+ down_read(&inode->i_alloc_sem);
*phys_block = mapping->a_ops->bmap(mapping, block);
+ up_read(&inode->i_alloc_sem);
+ mutex_unlock(&inode->i_mutex);
unlock_kernel();

return 0;

--

2007-10-29 13:36:55

by Chris Mason

Subject: Re: [patch 4/6][RFC] Attempt to plug race with truncate

On Fri, 26 Oct 2007 16:37:36 -0700
Mike Waychison <[email protected]> wrote:

> Attempt to deal with races with truncate paths.
>
> I'm not really sure on the locking here, but these seem to be taken
> by the truncate path. BKL is left as some filesystem may(?) still
> require it.
>
> Signed-off-by: Mike Waychison <[email protected]>
> fs/ioctl.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> Index: linux-2.6.23/fs/ioctl.c
> ===================================================================
> --- linux-2.6.23.orig/fs/ioctl.c 2007-10-26 15:27:29.000000000
> -0700 +++ linux-2.6.23/fs/ioctl.c 2007-10-26
> 16:16:28.000000000 -0700 @@ -43,13 +43,21 @@ static long
> do_ioctl(struct file *filp, static int do_fibmap(struct address_space
> *mapping, sector_t block, sector_t *phys_block)
> {
> + struct inode *inode = mapping->host;
> +
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
> if (!mapping->a_ops->bmap)
> return -EINVAL;
>
> lock_kernel();
> + /* Avoid races with truncate */
> + mutex_lock(&inode->i_mutex);
> + /* FIXME: Do we really need i_alloc_sem? */
> + down_read(&inode->i_alloc_sem);


i_alloc_sem will avoid races with filesystems filling holes inside
writepage (where i_mutex isn't held). I'd expect everyone to currently
give some consistent result (either the old value or the new but not
garbage), but I wouldn't expect taking the semaphore to hurt anything.

-chris