On Tue, 2012-09-18 at 15:17 +0300, Dan Carpenter wrote:
> Hello Catherine Sullivan,
>
> The patch 91fbd8f081e2: "ixgbe: added reg_ops file to debugfs" from
> Aug 10, 2012, leads to the following Smatch warnings:
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:76
> ixgbe_dbg_reg_ops_read() warn: 'len' returned from snprintf() might be
> larger than 256
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:77
> ixgbe_dbg_reg_ops_read() warn: copy_to/from_user() returns a positive
> value
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:78
> ixgbe_dbg_reg_ops_read() warn: maybe return -EFAULT instead of the
> bytes remaining?
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:105
> ixgbe_dbg_reg_ops_write() warn: copy_to/from_user() returns a positive
> value
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:106
> ixgbe_dbg_reg_ops_write() warn: maybe return -EFAULT instead of the
> bytes remaining?
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:190
> ixgbe_dbg_netdev_ops_read() warn: 'len' returned from snprintf() might
> be larger than 256
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:191
> ixgbe_dbg_netdev_ops_read() warn: copy_to/from_user() returns a
> positive value
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:192
> ixgbe_dbg_netdev_ops_read() warn: maybe return -EFAULT instead of the
> bytes remaining?
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:220
> ixgbe_dbg_netdev_ops_write() warn: copy_to/from_user() returns a
> positive value
> drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c:221
> ixgbe_dbg_netdev_ops_write() warn: maybe return -EFAULT instead of the
> bytes remaining?
>
> 60 static ssize_t ixgbe_dbg_reg_ops_read(struct file *filp, char
> __user *buffer,
> 61 size_t count, loff_t
> *ppos)
> 62 {
> 63 struct ixgbe_adapter *adapter = filp->private_data;
> 64 char buf[256];
> 65 int bytes_not_copied;
> 66 int len;
> 67
> 68 /* don't allow partial reads */
> 69 if (*ppos != 0)
> 70 return 0;
>
> Why not?
>
> 71
> 72 len = snprintf(buf, sizeof(buf), "%s: %s\n",
> 73 adapter->netdev->name,
> ixgbe_dbg_reg_ops_buf);
>
> In theory at least when we add the adapter name to the print then we
> would be over 256 bytes. len would be number of bytes we wanted to
> print if we had space. Use vscnprintf() because that returns the
> number
> of bytes printed (excluding the NUL the same as snprintf() does).
>
> 74 if (count < len)
> 75 return -ENOSPC;
>
> Normally we would just fill their buffer up to count.
>
> 76 bytes_not_copied = copy_to_user(buffer, buf, len);
> 77 if (bytes_not_copied < 0)
> 78 return bytes_not_copied;
>
> This test should be:
> if (copy_to_user(buffer, buf, len))
> return -EFAULT;
>
> 79
> 80 *ppos = len;
> 81 return len;
> 82 }
>
> Normally these functions just vscnprintf() to a buffer and then call
> simple_read_from_buffer(). Thanks for the report Dan. We will put together a patch to fix this up. Cheers, Jeff
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------------ Live Security Virtual Conference Exclusive live event will cover all the ways today's security and threat landscape has changed and how IT managers can respond. Discussions will include endpoint security, mobile security and the latest in malware threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________ E1000-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/e1000-devel To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
