[bug report] Driver for IBM System i/p VNIC protocol

[bug report] Driver for IBM System i/p VNIC protocol

[Dan Carpenter]

Hello Thomas Falcon,

The patch 032c5e82847a: "Driver for IBM System i/p VNIC protocol"
from Dec 21, 2015, leads to the following static checker warning:

	drivers/net/ethernet/ibm/ibmvnic.c:2957 error_level_write()
	why cast 'kstrtoul()?'

drivers/net/ethernet/ibm/ibmvnic.c
  2946  static ssize_t error_level_write(struct file *file, const char __user *user_buf,
  2947                                   size_t len, loff_t *ppos)
  2948  {
  2949          struct ibmvnic_fw_comp_internal *ras_comp_int = file->private_data;
  2950          struct ibmvnic_adapter *adapter = ras_comp_int->adapter;
  2951          int num = ras_comp_int->num;
  2952          union ibmvnic_crq crq;
  2953          unsigned long val;
  2954          char buff[9]; /* decimal max int plus \n and \0 */
  2955  
  2956          copy_from_user(buff, user_buf, sizeof(buff));

No error checking.

  2957          val = kstrtoul(buff, 10, NULL);

This is a wrong conversion from simple_strtoul().  The code has clearly
never been tested.  There are four other buggy untested calls to
kstrtoul() in this file.

  2958  
  2959          if (val > 9)
  2960                  val = 9;
  2961  
  2962          memset(&crq, 0, sizeof(crq));
  2963          crq.control_ras.first = IBMVNIC_CRQ_CMD;
  2964          crq.control_ras.cmd = CONTROL_RAS;
  2965          crq.control_ras.correlator = adapter->ras_comps[num].correlator;
  2966          crq.control_ras.op = IBMVNIC_ERROR_LEVEL;
  2967          crq.control_ras.level = val;
  2968          ibmvnic_send_crq(adapter, &crq);
  2969  
  2970          return len;
  2971  }

regards,
dan carpenter

Re: [bug report] Driver for IBM System i/p VNIC protocol

[Thomas Falcon]

On 11/16/2016 06:25 AM, Dan Carpenter wrote:
> Hello Thomas Falcon,
>
> The patch 032c5e82847a: "Driver for IBM System i/p VNIC protocol"
> from Dec 21, 2015, leads to the following static checker warning:
>
> 	drivers/net/ethernet/ibm/ibmvnic.c:2957 error_level_write()
> 	why cast 'kstrtoul()?'
>
> drivers/net/ethernet/ibm/ibmvnic.c
>   2946  static ssize_t error_level_write(struct file *file, const char __user *user_buf,
>   2947                                   size_t len, loff_t *ppos)
>   2948  {
>   2949          struct ibmvnic_fw_comp_internal *ras_comp_int = file->private_data;
>   2950          struct ibmvnic_adapter *adapter = ras_comp_int->adapter;
>   2951          int num = ras_comp_int->num;
>   2952          union ibmvnic_crq crq;
>   2953          unsigned long val;
>   2954          char buff[9]; /* decimal max int plus \n and \0 */
>   2955  
>   2956          copy_from_user(buff, user_buf, sizeof(buff));
>
> No error checking.
>
>   2957          val = kstrtoul(buff, 10, NULL);
>
> This is a wrong conversion from simple_strtoul().  The code has clearly
> never been tested.  There are four other buggy untested calls to
> kstrtoul() in this file.
>
>   2958  
>   2959          if (val > 9)
>   2960                  val = 9;
>   2961  
>   2962          memset(&crq, 0, sizeof(crq));
>   2963          crq.control_ras.first = IBMVNIC_CRQ_CMD;
>   2964          crq.control_ras.cmd = CONTROL_RAS;
>   2965          crq.control_ras.correlator = adapter->ras_comps[num].correlator;
>   2966          crq.control_ras.op = IBMVNIC_ERROR_LEVEL;
>   2967          crq.control_ras.level = val;
>   2968          ibmvnic_send_crq(adapter, &crq);
>   2969  
>   2970          return len;
>   2971  }
Thank you for your time and attention, Dan.  I had also noticed these errors, but did not immediately fix it after being told by our firmware team that support had been discontinued for these features.  I plan to remove/fix these soon.

Thanks again,
Tom


> regards,
> dan carpenter
>