From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-gw3-out.broadcom.com ([216.31.210.64]:57603 "EHLO mail-gw3-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbbCFPU1 (ORCPT ); Fri, 6 Mar 2015 10:20:27 -0500 Message-ID: <54F9C5B8.9080004@broadcom.com> (sfid-20150306_162029_498763_1D9FE04C) Date: Fri, 6 Mar 2015 16:20:24 +0100 From: Arend van Spriel MIME-Version: 1.0 To: Kalle Valo CC: Arend van Spriel , linux-wireless , Pontus Fuchs Subject: Re: [PATCH] brcmfmac: Perform bound checking on vendor command buffer References: <1425655123-18916-1-git-send-email-arend@broadcom.com> In-Reply-To: <1425655123-18916-1-git-send-email-arend@broadcom.com> Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Sender: linux-wireless-owner@vger.kernel.org List-ID: On 03/06/15 16:18, Arend van Spriel wrote: > From: Pontus Fuchs > > A short or malformed vendor command buffer could cause reads outside > the command buffer. > > Cc: stable@vger.kernel.org # v3.19 > Signed-off-by: Pontus Fuchs > [arend@broadcom.com: slightly modified debug trace output] > Signed-off-by: Arend van Spriel > --- Hi Kalle, Forgot to mention this is for v4.0 kernel. Regards, Arend > drivers/net/wireless/brcm80211/brcmfmac/vendor.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/brcm80211/brcmfmac/vendor.c > index 50cdf70..8eff275 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/vendor.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/vendor.c > @@ -39,13 +39,22 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy, > void *dcmd_buf = NULL, *wr_pointer; > u16 msglen, maxmsglen = PAGE_SIZE - 0x100; > > - brcmf_dbg(TRACE, "cmd %x set %d len %d\n", cmdhdr->cmd, cmdhdr->set, > - cmdhdr->len); > + if (len< sizeof(*cmdhdr)) { > + brcmf_err("vendor command too short: %d\n", len); > + return -EINVAL; > + } > > vif = container_of(wdev, struct brcmf_cfg80211_vif, wdev); > ifp = vif->ifp; > > - len -= sizeof(struct brcmf_vndr_dcmd_hdr); > + brcmf_dbg(TRACE, "ifidx=%d, cmd=%d\n", ifp->ifidx, cmdhdr->cmd); > + > + if (cmdhdr->offset> len) { > + brcmf_err("bad buffer offset %d> %d\n", cmdhdr->offset, len); > + return -EINVAL; > + } > + > + len -= cmdhdr->offset; > ret_len = cmdhdr->len; > if (ret_len> 0 || len> 0) { > if (len> BRCMF_DCMD_MAXLEN) {