Re: [PATCH] Staging: lustre: Added missing __user keyword to several struct fields

From: Greg Kroah-Hartman
Date: Mon Feb 09 2015 - 18:37:59 EST

Next message: David Daney: "[PATCH] stable_kernel_rules: Add clause about specification of kernel versions to patch."
Previous message: Greg Kroah-Hartman: "Re: Question regarding sparse warning in staging/lustre"
In reply to: Adrian Remonda: "[PATCH] Staging: lustre: Added missing __user keyword to several struct fields"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Feb 09, 2015 at 11:56:58PM +0100, Adrian Remonda wrote:
> This is a patch that fixes up missing __user warnings found by the sparse
>
> modified: drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> modified: drivers/staging/lustre/include/linux/lnet/lnetst.h
> modified: drivers/staging/lustre/lnet/selftest/conctl.c
> modified: drivers/staging/lustre/lnet/selftest/console.c
> modified: drivers/staging/lustre/lnet/selftest/console.h
>
> modified: drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> modified: drivers/staging/lustre/lnet/selftest/console.h
>
> Signed-off-by: Adrian Remonda <adrianremonda@xxxxxxxxx>
> ---
> .../lustre/include/linux/libcfs/libcfs_ioctl.h | 4 +--
> drivers/staging/lustre/include/linux/lnet/lnetst.h | 36 +++++++++++-----------
> drivers/staging/lustre/lnet/selftest/console.c | 2 +-
> drivers/staging/lustre/lnet/selftest/console.h | 4 ++-
> 4 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> index 3ee38782ad8c..aa687b79384b 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_ioctl.h
> @@ -61,9 +61,9 @@ struct libcfs_ioctl_data {
> char *ioc_inlbuf2;
>
> __u32 ioc_plen1; /* buffers in userspace */
> - char *ioc_pbuf1;
> + char __user *ioc_pbuf1;
> __u32 ioc_plen2; /* buffers in userspace */
> - char *ioc_pbuf2;
> + char __user *ioc_pbuf2;
>
> char ioc_bulk[0];
> };
> diff --git a/drivers/staging/lustre/include/linux/lnet/lnetst.h b/drivers/staging/lustre/include/linux/lnet/lnetst.h
> index 885f708d4031..846cddc11a81 100644
> --- a/drivers/staging/lustre/include/linux/lnet/lnetst.h
> +++ b/drivers/staging/lustre/include/linux/lnet/lnetst.h
> @@ -277,7 +277,7 @@ typedef struct {
> int lstio_dbg_timeout; /* IN: timeout of debug */
>
> int lstio_dbg_nmlen; /* IN: len of name */
> - char *lstio_dbg_namep; /* IN: name of group|batch */
> + char __user *lstio_dbg_namep; /* IN: name of group|batch */
> int lstio_dbg_count; /* IN: # of test nodes to debug */
> lnet_process_id_t *lstio_dbg_idsp; /* IN: id of test nodes */
> struct list_head *lstio_dbg_resultp; /* OUT: list head of result buffer */
> @@ -286,13 +286,13 @@ typedef struct {
> typedef struct {
> int lstio_grp_key; /* IN: session key */
> int lstio_grp_nmlen; /* IN: name length */
> - char *lstio_grp_namep; /* IN: group name */
> + char __user *lstio_grp_namep; /* IN: group name */
> } lstio_group_add_args_t;
>
> typedef struct {
> int lstio_grp_key; /* IN: session key */
> int lstio_grp_nmlen; /* IN: name length */
> - char *lstio_grp_namep; /* IN: group name */
> + char __user *lstio_grp_namep; /* IN: group name */
> } lstio_group_del_args_t;
>
> #define LST_GROUP_CLEAN 1 /* remove inactive nodes in the group */
> @@ -316,7 +316,7 @@ typedef struct {
> char *lstio_grp_namep; /* IN: group name */
> int lstio_grp_count; /* IN: # of nodes */
> /** OUT: session features */
> - unsigned *lstio_grp_featp;
> + unsigned __user *lstio_grp_featp;
> lnet_process_id_t *lstio_grp_idsp; /* IN: nodes */
> struct list_head *lstio_grp_resultp; /* OUT: list head of result buffer */
> } lstio_group_nodes_args_t;
> @@ -344,21 +344,21 @@ typedef struct {
> typedef struct {
> int lstio_bat_key; /* IN: session key */
> int lstio_bat_nmlen; /* IN: name length */
> - char *lstio_bat_namep; /* IN: batch name */
> + char __user *lstio_bat_namep; /* IN: batch name */
> } lstio_batch_add_args_t;
>
> typedef struct {
> int lstio_bat_key; /* IN: session key */
> int lstio_bat_nmlen; /* IN: name length */
> - char *lstio_bat_namep; /* IN: batch name */
> + char __user *lstio_bat_namep; /* IN: batch name */
> } lstio_batch_del_args_t;
>
> typedef struct {
> int lstio_bat_key; /* IN: session key */
> int lstio_bat_timeout; /* IN: timeout for the batch */
> int lstio_bat_nmlen; /* IN: name length */
> - char *lstio_bat_namep; /* IN: batch name */
> - struct list_head *lstio_bat_resultp; /* OUT: list head of result buffer */
> + char __user *lstio_bat_namep; /* IN: batch name */
> + struct list_head __user *lstio_bat_resultp; /* OUT: list head of result buffer */
> } lstio_batch_run_args_t;
>
> typedef struct {
> @@ -375,7 +375,7 @@ typedef struct {
> int lstio_bat_client; /* IN: is test client? */
> int lstio_bat_timeout; /* IN: timeout for waiting */
> int lstio_bat_nmlen; /* IN: name length */
> - char *lstio_bat_namep; /* IN: batch name */
> + char __user *lstio_bat_namep; /* IN: batch name */
> struct list_head *lstio_bat_resultp; /* OUT: list head of result buffer */
> } lstio_batch_query_args_t;
>
> @@ -389,13 +389,13 @@ typedef struct {
> typedef struct {
> int lstio_bat_key; /* IN: session key */
> int lstio_bat_nmlen; /* IN: name length */
> - char *lstio_bat_namep; /* IN: name */
> + char __user *lstio_bat_namep; /* IN: name */
> int lstio_bat_server; /* IN: query server or not */
> int lstio_bat_testidx; /* IN: test index */
> lstcon_test_batch_ent_t *lstio_bat_entp; /* OUT: batch ent */
>
> - int *lstio_bat_idxp; /* IN/OUT: index of node */
> - int *lstio_bat_ndentp; /* IN/OUT: # of nodent */
> + int __user *lstio_bat_idxp; /* IN/OUT: index of node */
> + int __user *lstio_bat_ndentp; /* IN/OUT: # of nodent */
> lstcon_node_ent_t *lstio_bat_dentsp; /* array of nodent */
> } lstio_batch_info_args_t;
>
> @@ -404,7 +404,7 @@ typedef struct {
> int lstio_sta_key; /* IN: session key */
> int lstio_sta_timeout; /* IN: timeout for stat request */
> int lstio_sta_nmlen; /* IN: group name length */
> - char *lstio_sta_namep; /* IN: group name */
> + char __user *lstio_sta_namep; /* IN: group name */
> int lstio_sta_count; /* IN: # of pid */
> lnet_process_id_t *lstio_sta_idsp; /* IN: pid */
> struct list_head *lstio_sta_resultp; /* OUT: list head of result buffer */
> @@ -421,7 +421,7 @@ typedef enum {
> typedef struct {
> int lstio_tes_key; /* IN: session key */
> int lstio_tes_bat_nmlen; /* IN: batch name len */
> - char *lstio_tes_bat_name; /* IN: batch name */
> + char __user *lstio_tes_bat_name; /* IN: batch name */
> int lstio_tes_type; /* IN: test type */
> int lstio_tes_oneside; /* IN: one sided test */
> int lstio_tes_loop; /* IN: loop count */
> @@ -430,16 +430,16 @@ typedef struct {
> int lstio_tes_dist; /* IN: node distribution in destination groups */
> int lstio_tes_span; /* IN: node span in destination groups */
> int lstio_tes_sgrp_nmlen; /* IN: source group name length */
> - char *lstio_tes_sgrp_name; /* IN: group name */
> + char __user *lstio_tes_sgrp_name; /* IN: group name */
> int lstio_tes_dgrp_nmlen; /* IN: destination group name length */
> - char *lstio_tes_dgrp_name; /* IN: group name */
> + char __user *lstio_tes_dgrp_name; /* IN: group name */
>
> int lstio_tes_param_len; /* IN: param buffer len */
> - void *lstio_tes_param; /* IN: parameter for specified test:
> + void __user *lstio_tes_param; /* IN: parameter for specified test:
> lstio_bulk_param_t,
> lstio_ping_param_t,
> ... more */
> - int *lstio_tes_retp; /* OUT: private returned value */
> + int __user *lstio_tes_retp; /* OUT: private returned value */
> struct list_head *lstio_tes_resultp; /* OUT: list head of result buffer */
> } lstio_test_args_t;
>
> diff --git a/drivers/staging/lustre/lnet/selftest/console.c b/drivers/staging/lustre/lnet/selftest/console.c
> index 3bb47fa5d5c3..129a4df6ca7c 100644
> --- a/drivers/staging/lustre/lnet/selftest/console.c
> +++ b/drivers/staging/lustre/lnet/selftest/console.c
> @@ -1767,7 +1767,7 @@ lstcon_session_new(char *name, int key, unsigned feats,
>
> int
> lstcon_session_info(lst_sid_t *sid_up, int *key_up, unsigned *featp,
> - lstcon_ndlist_ent_t *ndinfo_up, char *name_up, int len)
> + lstcon_ndlist_ent_t *ndinfo_up, char __user *name_up, int len)
> {
> lstcon_ndlist_ent_t *entp;
> lstcon_ndlink_t *ndl;
> diff --git a/drivers/staging/lustre/lnet/selftest/console.h b/drivers/staging/lustre/lnet/selftest/console.h
> index f960174ceff8..a9c26d4b5be0 100644
> --- a/drivers/staging/lustre/lnet/selftest/console.h
> +++ b/drivers/staging/lustre/lnet/selftest/console.h
> @@ -186,7 +186,7 @@ extern int lstcon_session_match(lst_sid_t sid);
> extern int lstcon_session_new(char *name, int key, unsigned version,
> int timeout, int flags, lst_sid_t *sid_up);
> extern int lstcon_session_info(lst_sid_t *sid_up, int *key, unsigned *verp,
> - lstcon_ndlist_ent_t *entp, char *name_up, int len);
> + lstcon_ndlist_ent_t *entp, __user char *name_up, int len);
> extern int lstcon_session_end(void);
> extern int lstcon_session_debug(int timeout, struct list_head *result_up);
> extern int lstcon_session_feats_check(unsigned feats);
> @@ -229,4 +229,6 @@ extern int lstcon_test_add(char *batch_name, int type, int loop,
> char *src_name, char *dst_name,
> void *param, int paramlen, int *retp,
> struct list_head *result_up);
> +extern int lstcon_ioctl_entry(unsigned int cmd, struct libcfs_ioctl_data *data);

This change doesn't look correct for this patch.

Also, I really don't want to take these as-is, as we don't have any way
to properly test them, and the whole user/kernel api is horribly
convoluted right now, hopefully it will get cleaned up eventually before
we merge it out of staging.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Next message: David Daney: "[PATCH] stable_kernel_rules: Add clause about specification of kernel versions to patch."
Previous message: Greg Kroah-Hartman: "Re: Question regarding sparse warning in staging/lustre"
In reply to: Adrian Remonda: "[PATCH] Staging: lustre: Added missing __user keyword to several struct fields"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]