Re: [PATCH] netxen: sparse warning and ioctl bug fixes
On Tue, Dec 05, 2006 at 12:07:17PM -0800, Stephen Hemminger wrote: > Fix sparse warnings and get rid of casts that hide misuse > of __user pointers. There were two real bugs here: > * ioctl was copying uninitialized stack om NETXEN_NIC_NAME > * ioctl was dereferencing a user pointer > in nettxen_nic_cmd_clear_stats. > > IMHO the ioctl usage in this driver is going to be more maintenance > burden than useful. and should be removed. Yes, please send a patch to kill the ioctls. Given all the other mess in the driver I really wonder who ACKed it going in. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] netxen: sparse warning and ioctl bug fixes
Stephen Hemminger wrote: Fix sparse warnings and get rid of casts that hide misuse of __user pointers. There were two real bugs here: * ioctl was copying uninitialized stack om NETXEN_NIC_NAME * ioctl was dereferencing a user pointer in nettxen_nic_cmd_clear_stats. IMHO the ioctl usage in this driver is going to be more maintenance burden than useful. and should be removed. Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]> ACK but netxen is in heavy flux right now, with upstream workqueue changes, changes from Amit, etc. so, it didn't apply. Ever considered submitting via git? :) Jeff - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] netxen: sparse warning and ioctl bug fixes
On Wed, 6 Dec 2006 21:40:34 +0530 "Amit S. Kale" <[EMAIL PROTECTED]> wrote: > Hi Stephen, > > This patch looks good. > > ioctls: NetXen chip is far more generic and will eventually show a > significant > amount of functionality in different products. We need ioctl analysis for > user level tools that can tell states of registers etc. Removing ioctls all > together will make any analysis from userland extremely difficult. Use (and extend) existing ethtool interface to get registers and statistics. > > Does anyone have ideas on improving this ioctl interface? > > Thanks. > -Amit > - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] netxen: sparse warning and ioctl bug fixes
Hi Stephen,
This patch looks good.
ioctls: NetXen chip is far more generic and will eventually show a significant
amount of functionality in different products. We need ioctl analysis for
user level tools that can tell states of registers etc. Removing ioctls all
together will make any analysis from userland extremely difficult.
Does anyone have ideas on improving this ioctl interface?
Thanks.
-Amit
On Wednesday 06 December 2006 01:37, Stephen Hemminger wrote:
> Fix sparse warnings and get rid of casts that hide misuse
> of __user pointers. There were two real bugs here:
> * ioctl was copying uninitialized stack om NETXEN_NIC_NAME
> * ioctl was dereferencing a user pointer
> in nettxen_nic_cmd_clear_stats.
>
> IMHO the ioctl usage in this driver is going to be more maintenance
> burden than useful. and should be removed.
>
> Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
> ---
> drivers/net/netxen/netxen_nic.h |5 +
> drivers/net/netxen/netxen_nic_hdr.h | 128
> + drivers/net/netxen/netxen_nic_init.c |
> 38 +-
> drivers/net/netxen/netxen_nic_ioctl.h |2 -
> drivers/net/netxen/netxen_nic_main.c | 34 +++--
> 5 files changed, 99 insertions(+), 108 deletions(-)
>
> diff --git a/drivers/net/netxen/netxen_nic.h
> b/drivers/net/netxen/netxen_nic.h index d925053..f66ebe3 100644
> --- a/drivers/net/netxen/netxen_nic.h
> +++ b/drivers/net/netxen/netxen_nic.h
> @@ -916,9 +916,8 @@ void netxen_tso_check(struct netxen_adap
> struct cmd_desc_type0 *desc, struct sk_buff *skb);
> int netxen_nic_hw_resources(struct netxen_adapter *adapter);
> void netxen_nic_clear_stats(struct netxen_adapter *adapter);
> -int
> -netxen_nic_do_ioctl(struct netxen_adapter *adapter, void *u_data,
> - struct netxen_port *port);
> +int netxen_nic_do_ioctl(struct netxen_adapter *adapter, void __user
> *u_data, +struct netxen_port *port);
> int netxen_nic_rx_has_work(struct netxen_adapter *adapter);
> int netxen_nic_tx_has_work(struct netxen_adapter *adapter);
> void netxen_watchdog_task(unsigned long v);
> diff --git a/drivers/net/netxen/netxen_nic_hdr.h
> b/drivers/net/netxen/netxen_nic_hdr.h index 72c6ec4..2461afc 100644
> --- a/drivers/net/netxen/netxen_nic_hdr.h
> +++ b/drivers/net/netxen/netxen_nic_hdr.h
> @@ -228,139 +228,139 @@ enum {
> /* This field defines CRB adr [31:20] of the agents */
>
> #define NETXEN_HW_CRB_HUB_AGT_ADR_MN \
> - ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MN_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MN_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_PH \
> - ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_PH_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_PH_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_MS \
> - ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MS_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MS_CRB_AGT_ADR)
>
> #define NETXEN_HW_CRB_HUB_AGT_ADR_PS \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_PS_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_PS_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_SS \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SS_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SS_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX3 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX3_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX3_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_QMS\
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_QMS_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_QMS_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_SQS0 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS0_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS0_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_SQS1 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS1_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS1_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_SQS2 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS2_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS2_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_SQS3 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS3_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS3_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_C2C0 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C0_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C0_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_C2C1 \
> - ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C1_CRB_AGT_ADR)
> + (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C1_CRB_AGT_ADR)
> #define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX2 \
> -
[PATCH] netxen: sparse warning and ioctl bug fixes
Fix sparse warnings and get rid of casts that hide misuse
of __user pointers. There were two real bugs here:
* ioctl was copying uninitialized stack om NETXEN_NIC_NAME
* ioctl was dereferencing a user pointer
in nettxen_nic_cmd_clear_stats.
IMHO the ioctl usage in this driver is going to be more maintenance
burden than useful. and should be removed.
Signed-off-by: Stephen Hemminger <[EMAIL PROTECTED]>
---
drivers/net/netxen/netxen_nic.h |5 +
drivers/net/netxen/netxen_nic_hdr.h | 128 +
drivers/net/netxen/netxen_nic_init.c | 38 +-
drivers/net/netxen/netxen_nic_ioctl.h |2 -
drivers/net/netxen/netxen_nic_main.c | 34 +++--
5 files changed, 99 insertions(+), 108 deletions(-)
diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index d925053..f66ebe3 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -916,9 +916,8 @@ void netxen_tso_check(struct netxen_adap
struct cmd_desc_type0 *desc, struct sk_buff *skb);
int netxen_nic_hw_resources(struct netxen_adapter *adapter);
void netxen_nic_clear_stats(struct netxen_adapter *adapter);
-int
-netxen_nic_do_ioctl(struct netxen_adapter *adapter, void *u_data,
- struct netxen_port *port);
+int netxen_nic_do_ioctl(struct netxen_adapter *adapter, void __user *u_data,
+ struct netxen_port *port);
int netxen_nic_rx_has_work(struct netxen_adapter *adapter);
int netxen_nic_tx_has_work(struct netxen_adapter *adapter);
void netxen_watchdog_task(unsigned long v);
diff --git a/drivers/net/netxen/netxen_nic_hdr.h
b/drivers/net/netxen/netxen_nic_hdr.h
index 72c6ec4..2461afc 100644
--- a/drivers/net/netxen/netxen_nic_hdr.h
+++ b/drivers/net/netxen/netxen_nic_hdr.h
@@ -228,139 +228,139 @@ enum {
/* This field defines CRB adr [31:20] of the agents */
#define NETXEN_HW_CRB_HUB_AGT_ADR_MN \
- ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MN_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MN_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_PH \
- ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_PH_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_PH_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_MS \
- ((NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MS_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H0_CH_HUB_ADR << 7) | NETXEN_HW_MS_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_PS \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_PS_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_PS_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_SS \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SS_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SS_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX3\
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX3_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX3_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_QMS \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_QMS_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_QMS_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_SQS0 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS0_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS0_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_SQS1 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS1_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS1_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_SQS2 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS2_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS2_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_SQS3 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS3_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_SQGS3_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_C2C0 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C0_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C0_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_C2C1 \
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C1_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_C2C1_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX2\
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX2_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX2_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX4\
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX4_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX4_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX7\
- ((NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX7_CRB_AGT_ADR)
+ (((u32)NETXEN_HW_H1_CH_HUB_ADR << 7) | NETXEN_HW_RPMX7_CRB_AGT_ADR)
#define NETXEN_HW_CRB_HUB_AGT_ADR_RPMX9\
- ((NETXEN
