lkml.org 
[lkml]   [2015]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 2/2] staging: lustre: replace ll_{get,put}name() with {get,put}name()
Date
On 2015/04/21, 9:50 PM, "Boqun Feng" <boqun.feng@gmail.com> wrote:

>As pointed by Al Viro:
>
>https://lkml.org/lkml/2015/4/11/243
>
>There are bugs in ll_getname() because of wrong assumptions of returning
>values from strncpy_from_user(). Moreover, what ll_getname want to do is
>just to try copy the file name from userland. Since we already have
>getname() for the same purpose, it's better to replace ll_getname() with
>getname(), so is ll_putname().
>
>Besides, remove unused code for checking whether namelen is 0 or not in
>case LL_IOC_REMOVE_ENTRY, because zero-length file name is already
>handled by getname() in the same way as ll_getname().
>
>Suggested-by: Al Viro <viro@ZenIV.linux.org.uk>
>Signed-off-by: Boqun Feng <boqun.feng@gmail.com>

Looks good, you can add my:
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>

>---
> drivers/staging/lustre/lustre/llite/dir.c | 60
>++++++----------------
> .../staging/lustre/lustre/llite/llite_internal.h | 2 +-
> drivers/staging/lustre/lustre/llite/namei.c | 2 +-
> 3 files changed, 18 insertions(+), 46 deletions(-)
>
>diff --git a/drivers/staging/lustre/lustre/llite/dir.c
>b/drivers/staging/lustre/lustre/llite/dir.c
>index a182019..c75fc38 100644
>--- a/drivers/staging/lustre/lustre/llite/dir.c
>+++ b/drivers/staging/lustre/lustre/llite/dir.c
>@@ -1216,30 +1216,6 @@ out:
> return rc;
> }
>
>-static char *
>-ll_getname(const char __user *filename)
>-{
>- int ret = 0, len;
>- char *tmp = __getname();
>-
>- if (!tmp)
>- return ERR_PTR(-ENOMEM);
>-
>- len = strncpy_from_user(tmp, filename, PATH_MAX);
>- if (len == 0)
>- ret = -ENOENT;
>- else if (len > PATH_MAX)
>- ret = -ENAMETOOLONG;
>-
>- if (ret) {
>- __putname(tmp);
>- tmp = ERR_PTR(ret);
>- }
>- return tmp;
>-}
>-
>-#define ll_putname(filename) __putname(filename)
>-
> static long ll_dir_ioctl(struct file *file, unsigned int cmd, unsigned
>long arg)
> {
> struct inode *inode = file_inode(file);
>@@ -1441,7 +1417,7 @@ free_lmv:
> return rc;
> }
> case LL_IOC_REMOVE_ENTRY: {
>- char *filename = NULL;
>+ struct filename *name = NULL;
> int namelen = 0;
> int rc;
>
>@@ -1453,20 +1429,16 @@ free_lmv:
> if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_LVB_TYPE))
> return -ENOTSUPP;
>
>- filename = ll_getname((const char *)arg);
>- if (IS_ERR(filename))
>- return PTR_ERR(filename);
>+ name = getname((const char *)arg);
>+ if (IS_ERR(name))
>+ return PTR_ERR(name);
>
>- namelen = strlen(filename);
>- if (namelen < 1) {
>- rc = -EINVAL;
>- goto out_rmdir;
>- }
>+ namelen = strlen(name->name);
>+
>+ rc = ll_rmdir_entry(inode, name->name, namelen);
>
>- rc = ll_rmdir_entry(inode, filename, namelen);
>-out_rmdir:
>- if (filename)
>- ll_putname(filename);
>+ if (name)
>+ putname(name);
> return rc;
> }
> case LL_IOC_LOV_SWAP_LAYOUTS:
>@@ -1481,16 +1453,16 @@ out_rmdir:
> struct lov_user_md *lump;
> struct lov_mds_md *lmm = NULL;
> struct mdt_body *body;
>- char *filename = NULL;
>+ struct filename *name = NULL;
> int lmmsize;
>
> if (cmd == IOC_MDC_GETFILEINFO ||
> cmd == IOC_MDC_GETFILESTRIPE) {
>- filename = ll_getname((const char *)arg);
>- if (IS_ERR(filename))
>- return PTR_ERR(filename);
>+ name = getname((const char *)arg);
>+ if (IS_ERR(name))
>+ return PTR_ERR(name);
>
>- rc = ll_lov_getstripe_ea_info(inode, filename, &lmm,
>+ rc = ll_lov_getstripe_ea_info(inode, name->name, &lmm,
> &lmmsize, &request);
> } else {
> rc = ll_dir_getstripe(inode, &lmm, &lmmsize, &request);
>@@ -1556,8 +1528,8 @@ skip_lmm:
>
> out_req:
> ptlrpc_req_finished(request);
>- if (filename)
>- ll_putname(filename);
>+ if (name)
>+ putname(name);
> return rc;
> }
> case IOC_LOV_GETINFO: {
>diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
>b/drivers/staging/lustre/lustre/llite/llite_internal.h
>index 2af1d72..0950565 100644
>--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
>+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
>@@ -714,7 +714,7 @@ struct inode *ll_iget(struct super_block *sb, ino_t
>hash,
> int ll_md_blocking_ast(struct ldlm_lock *, struct ldlm_lock_desc *,
> void *data, int flag);
> struct dentry *ll_splice_alias(struct inode *inode, struct dentry *de);
>-int ll_rmdir_entry(struct inode *dir, char *name, int namelen);
>+int ll_rmdir_entry(struct inode *dir, const char *name, int namelen);
>
> /* llite/rw.c */
> int ll_prepare_write(struct file *, struct page *, unsigned from,
>unsigned to);
>diff --git a/drivers/staging/lustre/lustre/llite/namei.c
>b/drivers/staging/lustre/lustre/llite/namei.c
>index 890ac19..ec48d8d 100644
>--- a/drivers/staging/lustre/lustre/llite/namei.c
>+++ b/drivers/staging/lustre/lustre/llite/namei.c
>@@ -867,7 +867,7 @@ static inline void ll_get_child_fid(struct dentry
>*child, struct lu_fid *fid)
> /**
> * Remove dir entry
> **/
>-int ll_rmdir_entry(struct inode *dir, char *name, int namelen)
>+int ll_rmdir_entry(struct inode *dir, const char *name, int namelen)
> {
> struct ptlrpc_request *request = NULL;
> struct md_op_data *op_data;
>--
>2.3.5
>
>


Cheers, Andreas
--
Andreas Dilger

Lustre Software Architect
Intel High Performance Data Division




\
 
 \ /
  Last update: 2015-04-22 08:21    [W:0.075 / U:0.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site