Skip to content

Commit

Permalink
NFS: Add correct bounds checking to NFSv2 locks
Browse files Browse the repository at this point in the history
NFSv2 file locking currently fails the Connectathon tests, because the
calls to the VFS locking code do not return an EINVAL error if the
struct file_lock overflows the 32-bit boundaries.

The problem is due to the fact that we occasionally call helpers from
fs/locks.c in order to avoid RPC calls to the server when we know that a
local process holds the lock. These helpers are, of course, always
64-bit enabled, so EINVAL is not returned in cases when it would if
the call had gone to the NLM code.

For consistency, we therefore add support for a bounds-checking helper.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
  • Loading branch information
Trond Myklebust authored and Trond Myklebust committed Jul 9, 2008
1 parent f3d47a3 commit 2116271
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
20 changes: 15 additions & 5 deletions fs/nfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ static int do_setlk(struct file *filp, int cmd, struct file_lock *fl)
static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)
{
struct inode * inode = filp->f_mapping->host;
int ret = -ENOLCK;

dprintk("NFS: nfs_lock(f=%s/%ld, t=%x, fl=%x, r=%Ld:%Ld)\n",
inode->i_sb->s_id, inode->i_ino,
Expand All @@ -602,13 +603,22 @@ static int nfs_lock(struct file *filp, int cmd, struct file_lock *fl)

/* No mandatory locks over NFS */
if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
return -ENOLCK;
goto out_err;

if (NFS_PROTO(inode)->lock_check_bounds != NULL) {
ret = NFS_PROTO(inode)->lock_check_bounds(fl);
if (ret < 0)
goto out_err;
}

if (IS_GETLK(cmd))
return do_getlk(filp, cmd, fl);
if (fl->fl_type == F_UNLCK)
return do_unlk(filp, cmd, fl);
return do_setlk(filp, cmd, fl);
ret = do_getlk(filp, cmd, fl);
else if (fl->fl_type == F_UNLCK)
ret = do_unlk(filp, cmd, fl);
else
ret = do_setlk(filp, cmd, fl);
out_err:
return ret;
}

/*
Expand Down
24 changes: 24 additions & 0 deletions fs/nfs/proc.c
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,29 @@ nfs_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
return nlmclnt_proc(NFS_SERVER(inode)->nlm_host, cmd, fl);
}

/* Helper functions for NFS lock bounds checking */
#define NFS_LOCK32_OFFSET_MAX ((__s32)0x7fffffffUL)
static int nfs_lock_check_bounds(const struct file_lock *fl)
{
__s32 start, end;

start = (__s32)fl->fl_start;
if ((loff_t)start != fl->fl_start)
goto out_einval;

if (fl->fl_end != OFFSET_MAX) {
end = (__s32)fl->fl_end;
if ((loff_t)end != fl->fl_end)
goto out_einval;
} else
end = NFS_LOCK32_OFFSET_MAX;

if (start < 0 || start > end)
goto out_einval;
return 0;
out_einval:
return -EINVAL;
}

const struct nfs_rpc_ops nfs_v2_clientops = {
.version = 2, /* protocol version */
Expand Down Expand Up @@ -633,4 +656,5 @@ const struct nfs_rpc_ops nfs_v2_clientops = {
.file_open = nfs_open,
.file_release = nfs_release,
.lock = nfs_proc_lock,
.lock_check_bounds = nfs_lock_check_bounds,
};
1 change: 1 addition & 0 deletions include/linux/nfs_xdr.h
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,7 @@ struct nfs_rpc_ops {
int (*file_open) (struct inode *, struct file *);
int (*file_release) (struct inode *, struct file *);
int (*lock)(struct file *, int, struct file_lock *);
int (*lock_check_bounds)(const struct file_lock *);
void (*clear_acl_cache)(struct inode *);
};

Expand Down

0 comments on commit 2116271

Please sign in to comment.