[PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kevin Cernekee]

In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
the length of rxframe is validated.  This could lead to uninitialized
data being accessed (but not printed).  Since we already have a
perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
and ch.chspec is not modified by decchspec(), avoid the extra
assignment and use ch.chspec in the debug print.

Suggested-by: Mattias Nissler <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


V1->V2: Clarify changelog re: whether the uninitialized data is printed.


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 2ce675ab40ef..1c450c0727cb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -1853,7 +1853,6 @@ s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
 	struct afx_hdl *afx_hdl = &p2p->afx_hdl;
 	struct brcmf_cfg80211_vif *vif = ifp->vif;
 	struct brcmf_rx_mgmt_data *rxframe = (struct brcmf_rx_mgmt_data *)data;
-	u16 chanspec = be16_to_cpu(rxframe->chanspec);
 	struct brcmu_chan ch;
 	u8 *mgmt_frame;
 	u32 mgmt_frame_len;
@@ -1906,7 +1905,7 @@ s32 brcmf_p2p_notify_rx_mgmt_p2p_probereq(struct brcmf_if *ifp,
 	cfg80211_rx_mgmt(&vif->wdev, freq, 0, mgmt_frame, mgmt_frame_len, 0);
 
 	brcmf_dbg(INFO, "mgmt_frame_len (%d) , e->datalen (%d), chanspec (%04x), freq (%d)\n",
-		  mgmt_frame_len, e->datalen, chanspec, freq);
+		  mgmt_frame_len, e->datalen, ch.chspec, freq);
 
 	return 0;
 }
-- 
2.14.1.581.gf28d330327-goog

[PATCH V2 2/3] brcmfmac: Delete redundant length check

[Kevin Cernekee]

brcmf_fweh_process_event() sets event->datalen to the
endian-swapped value of event_packet->msg.datalen, which is the
same as emsg.datalen.  This length is already validated in
brcmf_fweh_process_event(), so there is no need to check it
again upon dequeuing the event.

Suggested-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 5 -----
 1 file changed, 5 deletions(-)


V1->V2: Delete the check instead of moving it.


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 4eb1e1ce9ace..27e661fa356f 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -257,11 +257,6 @@ static void brcmf_fweh_event_worker(struct work_struct *work)
 		brcmf_dbg_hex_dump(BRCMF_EVENT_ON(), event->data,
 				   min_t(u32, emsg.datalen, 64),
 				   "event payload, len=%d\n", emsg.datalen);
-		if (emsg.datalen > event->datalen) {
-			brcmf_err("event invalid length header=%d, msg=%d\n",
-				  event->datalen, emsg.datalen);
-			goto event_free;
-		}
 
 		/* special handling of interface event */
 		if (event->code == BRCMF_E_IF) {
-- 
2.14.1.581.gf28d330327-goog

[PATCH V2 3/3] brcmfmac: Add check for short event packets

[Kevin Cernekee]

The length of the data in the received skb is currently passed into
brcmf_fweh_process_event() as packet_len, but this value is not checked.
event_packet should be followed by DATALEN bytes of additional event
data.  Ensure that the received packet actually contains at least
DATALEN bytes of additional data, to avoid copying uninitialized memory
into event->data.

Suggested-by: Mattias Nissler <mnissler@chromium.org>
Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


V1->V2: No change.


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index 27e661fa356f..28361bb865f3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -424,7 +424,8 @@ void brcmf_fweh_process_event(struct brcmf_pub *drvr,
 	if (code != BRCMF_E_IF && !fweh->evt_handler[code])
 		return;
 
-	if (datalen > BRCMF_DCMD_MAXLEN)
+	if (datalen > BRCMF_DCMD_MAXLEN ||
+	    datalen + sizeof(*event_packet) < packet_len)
 		return;
 
 	if (in_interrupt())
-- 
2.14.1.581.gf28d330327-goog

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Arend van Spriel]

On 09-09-17 21:30, Kevin Cernekee wrote:
> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
> the length of rxframe is validated.  This could lead to uninitialized
> data being accessed (but not printed).  Since we already have a
> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
> and ch.chspec is not modified by decchspec(), avoid the extra
> assignment and use ch.chspec in the debug print.
> 
> Suggested-by: Mattias Nissler <mnissler@chromium.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> ---
>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> V1->V2: Clarify changelog re: whether the uninitialized data is printed.

This patch and the others in this series look fine to me.

Thanks,
Arend

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 09-09-17 21:30, Kevin Cernekee wrote:
>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>> the length of rxframe is validated.  This could lead to uninitialized
>> data being accessed (but not printed).  Since we already have a
>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>> and ch.chspec is not modified by decchspec(), avoid the extra
>> assignment and use ch.chspec in the debug print.
>>
>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>> ---
>>   drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>>
>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>
> This patch and the others in this series look fine to me.

Should these go to v4.14?

-- 
Kalle Valo

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Arend van Spriel]

On 9/12/2017 7:48 AM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 09-09-17 21:30, Kevin Cernekee wrote:
>>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>>> the length of rxframe is validated.  This could lead to uninitialized
>>> data being accessed (but not printed).  Since we already have a
>>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>>> and ch.chspec is not modified by decchspec(), avoid the extra
>>> assignment and use ch.chspec in the debug print.
>>>
>>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>> ---
>>>    drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>>
>>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>>
>> This patch and the others in this series look fine to me.
>
> Should these go to v4.14?

I have no strong opinion. These are certainly improvements, but it does 
not seem an -rc fix to me. Within this series I would say patch 3/3 adds 
an additional sanity check in the event processing against an attack so 
you may consider adding just that one to v4.14 and tag it for stable, ie.:

Cc: stable@vger.kernel.org # v3.8.x

Regards,
Arend

Re: [V2,3/3] brcmfmac: Add check for short event packets

[Kalle Valo]

Kevin Cernekee <cernekee@chromium.org> wrote:

> The length of the data in the received skb is currently passed into
> brcmf_fweh_process_event() as packet_len, but this value is not checked.
> event_packet should be followed by DATALEN bytes of additional event
> data.  Ensure that the received packet actually contains at least
> DATALEN bytes of additional data, to avoid copying uninitialized memory
> into event->data.
> 
> Suggested-by: Mattias Nissler <mnissler@chromium.org>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>

I'll queue this for v4.14 and add:

Cc: stable@vger.kernel.org # v3.8+

-- 
https://patchwork.kernel.org/patch/9945427/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 9/12/2017 7:48 AM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 09-09-17 21:30, Kevin Cernekee wrote:
>>>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>>>> the length of rxframe is validated.  This could lead to uninitialized
>>>> data being accessed (but not printed).  Since we already have a
>>>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>>>> and ch.chspec is not modified by decchspec(), avoid the extra
>>>> assignment and use ch.chspec in the debug print.
>>>>
>>>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>>>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>> ---
>>>>    drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>>>    1 file changed, 1 insertion(+), 2 deletions(-)
>>>>
>>>>
>>>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>>>
>>> This patch and the others in this series look fine to me.
>>
>> Should these go to v4.14?
>
> I have no strong opinion. These are certainly improvements, but it
> does not seem an -rc fix to me. Within this series I would say patch
> 3/3 adds an additional sanity check in the event processing against an
> attack so you may consider adding just that one to v4.14

Ok, I'll queue patch 3 to v4.14.

> and tag it for stable, ie.:
>
> Cc: stable@vger.kernel.org # v3.8.x

But why v3.8.x? I admit that I haven't fully figured out the stable tags
yet, but doesn't that mean that it will be only applied to v3.8.x and
nothing else? I was expecting it to be:

Cc: stable@vger.kernel.org # v3.8+

-- 
Kalle Valo

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Arend van Spriel]

On 9/12/2017 9:47 AM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 9/12/2017 7:48 AM, Kalle Valo wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>> On 09-09-17 21:30, Kevin Cernekee wrote:
>>>>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>>>>> the length of rxframe is validated.  This could lead to uninitialized
>>>>> data being accessed (but not printed).  Since we already have a
>>>>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>>>>> and ch.chspec is not modified by decchspec(), avoid the extra
>>>>> assignment and use ch.chspec in the debug print.
>>>>>
>>>>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>>>>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>>> ---
>>>>>     drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>>>>     1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>
>>>>>
>>>>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>>>>
>>>> This patch and the others in this series look fine to me.
>>>
>>> Should these go to v4.14?
>>
>> I have no strong opinion. These are certainly improvements, but it
>> does not seem an -rc fix to me. Within this series I would say patch
>> 3/3 adds an additional sanity check in the event processing against an
>> attack so you may consider adding just that one to v4.14
>
> Ok, I'll queue patch 3 to v4.14.
>
>> and tag it for stable, ie.:
>>
>> Cc: stable@vger.kernel.org # v3.8.x
>
> But why v3.8.x? I admit that I haven't fully figured out the stable tags
> yet, but doesn't that mean that it will be only applied to v3.8.x and
> nothing else? I was expecting it to be:
>
> Cc: stable@vger.kernel.org # v3.8+
>

It is actually in the stable-kernel-rules documentation [1]:

"""
Also, some patches may have kernel version prerequisites.  This can be
specified in the following format in the sign-off area:

.. code-block:: none

      Cc: <stable@vger.kernel.org> # 3.3.x

The tag has the meaning of:

.. code-block:: none

      git cherry-pick <this commit>

For each "-stable" tree starting with the specified version.
"""

The event handling code was added in v3.8.

Regards,
Arend

[1] https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kalle Valo]

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 9/12/2017 9:47 AM, Kalle Valo wrote:
>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>
>>> On 9/12/2017 7:48 AM, Kalle Valo wrote:
>>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>>
>>>>> On 09-09-17 21:30, Kevin Cernekee wrote:
>>>>>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>>>>>> the length of rxframe is validated.  This could lead to uninitialized
>>>>>> data being accessed (but not printed).  Since we already have a
>>>>>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>>>>>> and ch.chspec is not modified by decchspec(), avoid the extra
>>>>>> assignment and use ch.chspec in the debug print.
>>>>>>
>>>>>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>>>>>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>>>> ---
>>>>>>     drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>>>>>     1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>
>>>>>>
>>>>>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>>>>>
>>>>> This patch and the others in this series look fine to me.
>>>>
>>>> Should these go to v4.14?
>>>
>>> I have no strong opinion. These are certainly improvements, but it
>>> does not seem an -rc fix to me. Within this series I would say patch
>>> 3/3 adds an additional sanity check in the event processing against an
>>> attack so you may consider adding just that one to v4.14
>>
>> Ok, I'll queue patch 3 to v4.14.
>>
>>> and tag it for stable, ie.:
>>>
>>> Cc: stable@vger.kernel.org # v3.8.x
>>
>> But why v3.8.x? I admit that I haven't fully figured out the stable tags
>> yet, but doesn't that mean that it will be only applied to v3.8.x and
>> nothing else? I was expecting it to be:
>>
>> Cc: stable@vger.kernel.org # v3.8+
>>
>
> It is actually in the stable-kernel-rules documentation [1]:
>
> """
> Also, some patches may have kernel version prerequisites.  This can be
> specified in the following format in the sign-off area:
>
> .. code-block:: none
>
>      Cc: <stable@vger.kernel.org> # 3.3.x
>
> The tag has the meaning of:
>
> .. code-block:: none
>
>      git cherry-pick <this commit>
>
> For each "-stable" tree starting with the specified version.
> """

Yeah, but it says "starting with" which I interpret as "starting with
string '3.3'". For example the commit here would be applied to 3.3.1,
3.3.2 and 3.3.3 etc but _not_ to 3.4, 3.4.1, 3.5 or any later release.

Of course I can be way off here, wouldn't be the first :)

-- 
Kalle Valo

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Arend van Spriel]

+ Greg KH

On 9/12/2017 10:05 AM, Kalle Valo wrote:
> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>
>> On 9/12/2017 9:47 AM, Kalle Valo wrote:
>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>
>>>> On 9/12/2017 7:48 AM, Kalle Valo wrote:
>>>>> Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>>>>>
>>>>>> On 09-09-17 21:30, Kevin Cernekee wrote:
>>>>>>> In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
>>>>>>> the length of rxframe is validated.  This could lead to uninitialized
>>>>>>> data being accessed (but not printed).  Since we already have a
>>>>>>> perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
>>>>>>> and ch.chspec is not modified by decchspec(), avoid the extra
>>>>>>> assignment and use ch.chspec in the debug print.
>>>>>>>
>>>>>>> Suggested-by: Mattias Nissler <mnissler@chromium.org>
>>>>>>> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
>>>>>>> Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
>>>>>>> ---
>>>>>>>      drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
>>>>>>>      1 file changed, 1 insertion(+), 2 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>> V1->V2: Clarify changelog re: whether the uninitialized data is printed.
>>>>>>
>>>>>> This patch and the others in this series look fine to me.
>>>>>
>>>>> Should these go to v4.14?
>>>>
>>>> I have no strong opinion. These are certainly improvements, but it
>>>> does not seem an -rc fix to me. Within this series I would say patch
>>>> 3/3 adds an additional sanity check in the event processing against an
>>>> attack so you may consider adding just that one to v4.14
>>>
>>> Ok, I'll queue patch 3 to v4.14.
>>>
>>>> and tag it for stable, ie.:
>>>>
>>>> Cc: stable@vger.kernel.org # v3.8.x
>>>
>>> But why v3.8.x? I admit that I haven't fully figured out the stable tags
>>> yet, but doesn't that mean that it will be only applied to v3.8.x and
>>> nothing else? I was expecting it to be:
>>>
>>> Cc: stable@vger.kernel.org # v3.8+
>>>
>>
>> It is actually in the stable-kernel-rules documentation [1]:
>>
>> """
>> Also, some patches may have kernel version prerequisites.  This can be
>> specified in the following format in the sign-off area:
>>
>> .. code-block:: none
>>
>>       Cc: <stable@vger.kernel.org> # 3.3.x
>>
>> The tag has the meaning of:
>>
>> .. code-block:: none
>>
>>       git cherry-pick <this commit>
>>
>> For each "-stable" tree starting with the specified version.
>> """
>
> Yeah, but it says "starting with" which I interpret as "starting with
> string '3.3'". For example the commit here would be applied to 3.3.1,
> 3.3.2 and 3.3.3 etc but _not_ to 3.4, 3.4.1, 3.5 or any later release.
>
> Of course I can be way off here, wouldn't be the first :)

Dito. I interpret "each -stable tree" as each stable branch in the 
stable repository. Would Greg know?

Regards,
Arend

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Greg KH]

On Tue, Sep 12, 2017 at 10:18:00AM +0200, Arend van Spriel wrote:
> + Greg KH
> 
> On 9/12/2017 10:05 AM, Kalle Valo wrote:
> > Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> > 
> > > On 9/12/2017 9:47 AM, Kalle Valo wrote:
> > > > Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> > > > 
> > > > > On 9/12/2017 7:48 AM, Kalle Valo wrote:
> > > > > > Arend van Spriel <arend.vanspriel@broadcom.com> writes:
> > > > > > 
> > > > > > > On 09-09-17 21:30, Kevin Cernekee wrote:
> > > > > > > > In brcmf_p2p_notify_rx_mgmt_p2p_probereq(), chanspec is assigned before
> > > > > > > > the length of rxframe is validated.  This could lead to uninitialized
> > > > > > > > data being accessed (but not printed).  Since we already have a
> > > > > > > > perfectly good endian-swapped copy of rxframe->chanspec in ch.chspec,
> > > > > > > > and ch.chspec is not modified by decchspec(), avoid the extra
> > > > > > > > assignment and use ch.chspec in the debug print.
> > > > > > > > 
> > > > > > > > Suggested-by: Mattias Nissler <mnissler@chromium.org>
> > > > > > > > Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> > > > > > > > Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> > > > > > > > ---
> > > > > > > >      drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
> > > > > > > >      1 file changed, 1 insertion(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > 
> > > > > > > > V1->V2: Clarify changelog re: whether the uninitialized data is printed.
> > > > > > > 
> > > > > > > This patch and the others in this series look fine to me.
> > > > > > 
> > > > > > Should these go to v4.14?
> > > > > 
> > > > > I have no strong opinion. These are certainly improvements, but it
> > > > > does not seem an -rc fix to me. Within this series I would say patch
> > > > > 3/3 adds an additional sanity check in the event processing against an
> > > > > attack so you may consider adding just that one to v4.14
> > > > 
> > > > Ok, I'll queue patch 3 to v4.14.
> > > > 
> > > > > and tag it for stable, ie.:
> > > > > 
> > > > > Cc: stable@vger.kernel.org # v3.8.x
> > > > 
> > > > But why v3.8.x? I admit that I haven't fully figured out the stable tags
> > > > yet, but doesn't that mean that it will be only applied to v3.8.x and
> > > > nothing else? I was expecting it to be:
> > > > 
> > > > Cc: stable@vger.kernel.org # v3.8+
> > > > 
> > > 
> > > It is actually in the stable-kernel-rules documentation [1]:
> > > 
> > > """
> > > Also, some patches may have kernel version prerequisites.  This can be
> > > specified in the following format in the sign-off area:
> > > 
> > > .. code-block:: none
> > > 
> > >       Cc: <stable@vger.kernel.org> # 3.3.x
> > > 
> > > The tag has the meaning of:
> > > 
> > > .. code-block:: none
> > > 
> > >       git cherry-pick <this commit>
> > > 
> > > For each "-stable" tree starting with the specified version.
> > > """
> > 
> > Yeah, but it says "starting with" which I interpret as "starting with
> > string '3.3'". For example the commit here would be applied to 3.3.1,
> > 3.3.2 and 3.3.3 etc but _not_ to 3.4, 3.4.1, 3.5 or any later release.
> > 
> > Of course I can be way off here, wouldn't be the first :)
> 
> Dito. I interpret "each -stable tree" as each stable branch in the stable
> repository. Would Greg know?

"# 3.8+" and "# 3.8" mean the same thing to me, we would never backport
something to only a very specific kernel version, and leave newer kernel
versions to not have that fix.  That would be crazy, and would break our
"no regressions" rule (i.e. newer kernels should always work as good as
older kernels.)

Don't get hung up on the semantics here people, it's not all that
complicated, and I do it all by hand anyway :)

thanks,

greg k-h

Re: [PATCH V2 1/3] brcmfmac: Avoid possible out-of-bounds read

[Kalle Valo]

Greg KH <greg@kroah.com> writes:

> On Tue, Sep 12, 2017 at 10:18:00AM +0200, Arend van Spriel wrote:
>> + Greg KH
>> 
>> On 9/12/2017 10:05 AM, Kalle Valo wrote:
>> > Arend van Spriel <arend.vanspriel@broadcom.com> writes:
>> > 
>> > > It is actually in the stable-kernel-rules documentation [1]:
>> > > 
>> > > """
>> > > Also, some patches may have kernel version prerequisites.  This can be
>> > > specified in the following format in the sign-off area:
>> > > 
>> > > .. code-block:: none
>> > > 
>> > >       Cc: <stable@vger.kernel.org> # 3.3.x
>> > > 
>> > > The tag has the meaning of:
>> > > 
>> > > .. code-block:: none
>> > > 
>> > >       git cherry-pick <this commit>
>> > > 
>> > > For each "-stable" tree starting with the specified version.
>> > > """
>> > 
>> > Yeah, but it says "starting with" which I interpret as "starting with
>> > string '3.3'". For example the commit here would be applied to 3.3.1,
>> > 3.3.2 and 3.3.3 etc but _not_ to 3.4, 3.4.1, 3.5 or any later release.
>> > 
>> > Of course I can be way off here, wouldn't be the first :)
>> 
>> Dito. I interpret "each -stable tree" as each stable branch in the stable
>> repository. Would Greg know?
>
> "# 3.8+" and "# 3.8" mean the same thing to me, we would never backport
> something to only a very specific kernel version, and leave newer kernel
> versions to not have that fix.  That would be crazy, and would break our
> "no regressions" rule (i.e. newer kernels should always work as good as
> older kernels.)

Indeed, that would be crazy. Didn't think it like that, thanks for the
clarification.

> Don't get hung up on the semantics here people, it's not all that
> complicated, and I do it all by hand anyway :)

Manually? Oh man, that has to be so hard. I cannot understand how you
can do it.

-- 
Kalle Valo