| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <D15C917E.EBBB9%andreas.dilger@intel.com>
Date: Wed, 22 Apr 2015 05:45:55 +0000
From: "Dilger, Andreas" <andreas.dilger@...el.com>
To: Boqun Feng <boqun.feng@...il.com>,
"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: Al Viro <viro@...iv.linux.org.uk>,
"Drokin, Oleg" <oleg.drokin@...el.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Jan Kara <jack@...e.cz>
Subject: Re: [PATCH 2/2] staging: lustre: replace ll_{get,put}name() with
{get,put}name()
On 2015/04/21, 9:50 PM, "Boqun Feng" <boqun.feng@...il.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@...IV.linux.org.uk>
>Signed-off-by: Boqun Feng <boqun.feng@...il.com>
Looks good, you can add my:
Reviewed-by: Andreas Dilger <andreas.dilger@...el.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists