wli1271: buffer overflow static checker warning

wli1271: buffer overflow static checker warning

[Dan Carpenter]

Hello Luciano Coelho,

The patch f5fc0f86b02a: "wl1271: add wl1271 driver files" from Aug 6,
2009, leads to the following static checker warning:

	drivers/net/wireless/ti/wlcore/cmd.c:894 wlcore_cmd_configure_failsafe()
	warn: is 'buf' large enough for 'struct acx_header'?

drivers/net/wireless/ti/wlcore/cmd.c
   886  int wlcore_cmd_configure_failsafe(struct wl1271 *wl, u16 id, void *buf,
   887                                    size_t len, unsigned long valid_rets)
   888  {
   889          struct acx_header *acx = buf;
   890          int ret;
   891  
   892          wl1271_debug(DEBUG_CMD, "cmd configure (%d)", id);
   893  
   894          acx->id = cpu_to_le16(id);

"len" is the size of the "buf" buffer.

The warning is because wl1271_tm_cmd_test() and friends check if
len is too large but they don't check if it's too small.

   895  
   896          /* payload length, does not include any headers */
   897          acx->len = cpu_to_le16(len - sizeof(*acx));
   898  
   899          ret = wlcore_cmd_send_failsafe(wl, CMD_CONFIGURE, acx, len, 0,
   900                                         valid_rets);
   901          if (ret < 0) {
   902                  wl1271_warning("CONFIGURE command NOK");
   903                  return ret;
   904          }
   905  
   906          return ret;
   907  }

See also:

	drivers/net/wireless/ti/wl1251/cmd.c:29 wl1251_cmd_send()
	warn: is 'buf' large enough for 'struct wl1251_cmd_header'?

regards,
dan carpenter

Re: wli1271: buffer overflow static checker warning

[Luca Coelho]

On November 7, 2014 12:05:43 PM EET, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>Hello Luciano Coelho,
>
>The patch f5fc0f86b02a: "wl1271: add wl1271 driver files" from Aug 6,
>2009, leads to the following static checker warning:
 
2009?! :)
Does this code even still exist? :P

[PATCH] wlcore: check minimum buffer size in some cmd_send functions

[Luca Coelho]

From: Luciano Coelho <luciano.coelho@intel.com>

Check for the minimum required buffer length in wlcore_cmd_send() and
wlcore_cmd_configure_failsafe.  This ensures that we will never try to
use a buffer that is smaller than the required header.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
---

NOTE: this is only compile-tested.

drivers/net/wireless/ti/wlcore/cmd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ti/wlcore/cmd.c b/drivers/net/wireless/ti/wlcore/cmd.c
index 05604ee..5c6f3c8 100644
--- a/drivers/net/wireless/ti/wlcore/cmd.c
+++ b/drivers/net/wireless/ti/wlcore/cmd.c
@@ -64,6 +64,9 @@ static int __wlcore_cmd_send(struct wl1271 *wl, u16 id, void *buf,
 		     id != CMD_STOP_FWLOGGER))
 		return -EIO;
 
+	if (WARN_ON_ONCE(len < sizeof(*cmd)))
+		return -EIO;
+
 	cmd = buf;
 	cmd->id = cpu_to_le16(id);
 	cmd->status = 0;
@@ -891,6 +894,9 @@ int wlcore_cmd_configure_failsafe(struct wl1271 *wl, u16 id, void *buf,
 
 	wl1271_debug(DEBUG_CMD, "cmd configure (%d)", id);
 
+	if (WARN_ON_ONCE(len < sizeof(*acx)))
+		return -EIO;
+
 	acx->id = cpu_to_le16(id);
 
 	/* payload length, does not include any headers */
-- 
2.1.1