On Fri, Dec 05, 2014 at 12:03:47AM -0800, Tristan Lelong wrote:
> This patch fix a sparse warning in lustre sources
>
> warning: incorrect type in argument 1 (different address spaces)
> expected void [noderef] <asn:1>*to
> got char *<noident>
>
> This is done by adding the missing __user attribute on userland pointers inside
> the LPROC_SEQ_FOPS-like macros:
> - LPROC_SEQ_FOPS
> - LPROC_SEQ_FOPS_RW_TYPE
> - LPROC_SEQ_FOPS_WR_ONLY
> - LDLM_POOL_PROC_WRITER
>
> The patch also updates all the functions that are used by this macro:
> - lprocfs_wr_*
> - *_seq_write
>
> as well as some helpers used by the previously modified functions (otherwise
> fixing the sparse warning add some new ones):
> - lprocfs_write_frac_helper
> - lprocfs_write_helper
> - lprocfs_write_u64_helper
>
> The patch also fixes one __user pointer direct dereference by strncmp
> in function fld_proc_hash_seq_write by adding the proper copy_from_user.
>
> Signed-off-by: Tristan Lelong <tristan@xxxxxxxxxx>
> ---
> drivers/staging/lustre/lustre/fld/lproc_fld.c | 14 ++++--
> .../staging/lustre/lustre/include/lprocfs_status.h | 44 +++++++++--------
> drivers/staging/lustre/lustre/ldlm/ldlm_internal.h | 5 +-
> drivers/staging/lustre/lustre/ldlm/ldlm_pool.c | 4 +-
> drivers/staging/lustre/lustre/ldlm/ldlm_resource.c | 7 +--
> drivers/staging/lustre/lustre/lov/lproc_lov.c | 20 +++++---
> drivers/staging/lustre/lustre/mdc/lproc_mdc.c | 7 +--
> .../lustre/lustre/obdclass/linux/linux-module.c | 5 +-
> .../lustre/lustre/obdclass/lprocfs_status.c | 2 +-
> drivers/staging/lustre/lustre/osc/lproc_osc.c | 57 +++++++++++++---------
> .../staging/lustre/lustre/ptlrpc/lproc_ptlrpc.c | 25 +++++-----
> 11 files changed, 114 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/fld/lproc_fld.c b/drivers/staging/lustre/lustre/fld/lproc_fld.c
> index 95e7de1..9f1db6c 100644
> --- a/drivers/staging/lustre/lustre/fld/lproc_fld.c
> +++ b/drivers/staging/lustre/lustre/fld/lproc_fld.c
> @@ -87,13 +87,21 @@ fld_proc_hash_seq_show(struct seq_file *m, void *unused)
> }
>
> static ssize_t
> -fld_proc_hash_seq_write(struct file *file, const char *buffer,
> - size_t count, loff_t *off)
> +fld_proc_hash_seq_write(struct file *file,
> + const char __user *buffer,
> + size_t count, loff_t *off)
> {
> struct lu_client_fld *fld;
> struct lu_fld_hash *hash = NULL;
> + char name[80];
> int i;
>
> + if (count > 80)
> + return -ENAMETOOLONG;
> +
> + if (copy_from_user(name, buffer, count) != 0)
> + return -EFAULT;
How was this code ever working before?
And I know Joe asked, but how do you know that 80 is ok? And why on the
stack?
Shouldn't you just compare count to strlen(fld_hash[i].fh_name)? like you
do later on?
> +
> fld = ((struct seq_file *)file->private_data)->private;
> LASSERT(fld != NULL);
>
> @@ -101,7 +109,7 @@ fld_proc_hash_seq_write(struct file *file, const char *buffer,
> if (count != strlen(fld_hash[i].fh_name))
> continue;
>
> - if (!strncmp(fld_hash[i].fh_name, buffer, count)) {
> + if (!strncmp(fld_hash[i].fh_name, name, count)) {
So right now the code is just accessing user memory directly?
Seriously? Ugh.
Anyway, I don't like large stack variables like this, can you make it
dynamic instead?
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
