[SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

Joseph Salisbury-3
BugLink: https://bugs.launchpad.net/bugs/1805802

== SRU Justification ==
The response for a SNMP request can consist of multiple parts,
              which the cmd callback stages into a kernel buffer until all
              parts have been received. If the callback detects that the
              staging buffer provides insufficient space, it bails out with
              error.
              This processing is buggy for the first part of the response -
              while it initially checks for a length of 'data_len', it later
              copies an additional amount of
              'offsetof(struct qeth_snmp_cmd, data)' bytes.


== Fix ==
9a764c1e5968 ("s390/qeth: fix length check in SNMP processing")

== Regression Potential ==
Low.  Changes limited to s390.

== Test Case ==
A test kernel was built with this patch and tested by the original bug reporter.
The bug reporter states the test kernel resolved the bug.

Julian Wiedmann (1):
  s390/qeth: fix length check in SNMP processing

 drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU][Bionic][Cosmic][Disco][PATCH 1/1] s390/qeth: fix length check in SNMP processing

Joseph Salisbury-3
From: Julian Wiedmann <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1805802

The response for a SNMP request can consist of multiple parts, which
the cmd callback stages into a kernel buffer until all parts have been
received. If the callback detects that the staging buffer provides
insufficient space, it bails out with error.
This processing is buggy for the first part of the response - while it
initially checks for a length of 'data_len', it later copies an
additional amount of 'offsetof(struct qeth_snmp_cmd, data)' bytes.

Fix the calculation of 'data_len' for the first part of the response.
This also nicely cleans up the memcpy code.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Julian Wiedmann <[hidden email]>
Reviewed-by: Ursula Braun <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 9a764c1e59684c0358e16ccaafd870629f2cfe67)
Signed-off-by: Joseph Salisbury <[hidden email]>
---
 drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index b828345..b043620 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4513,8 +4513,8 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
 {
  struct qeth_ipa_cmd *cmd;
  struct qeth_arp_query_info *qinfo;
- struct qeth_snmp_cmd *snmp;
  unsigned char *data;
+ void *snmp_data;
  __u16 data_len;
 
  QETH_CARD_TEXT(card, 3, "snpcmdcb");
@@ -4522,7 +4522,6 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
  cmd = (struct qeth_ipa_cmd *) sdata;
  data = (unsigned char *)((char *)cmd - reply->offset);
  qinfo = (struct qeth_arp_query_info *) reply->param;
- snmp = &cmd->data.setadapterparms.data.snmp;
 
  if (cmd->hdr.return_code) {
  QETH_CARD_TEXT_(card, 4, "scer1%x", cmd->hdr.return_code);
@@ -4535,10 +4534,15 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
  return 0;
  }
  data_len = *((__u16 *)QETH_IPA_PDU_LEN_PDU1(data));
- if (cmd->data.setadapterparms.hdr.seq_no == 1)
- data_len -= (__u16)((char *)&snmp->data - (char *)cmd);
- else
- data_len -= (__u16)((char *)&snmp->request - (char *)cmd);
+ if (cmd->data.setadapterparms.hdr.seq_no == 1) {
+ snmp_data = &cmd->data.setadapterparms.data.snmp;
+ data_len -= offsetof(struct qeth_ipa_cmd,
+     data.setadapterparms.data.snmp);
+ } else {
+ snmp_data = &cmd->data.setadapterparms.data.snmp.request;
+ data_len -= offsetof(struct qeth_ipa_cmd,
+     data.setadapterparms.data.snmp.request);
+ }
 
  /* check if there is enough room in userspace */
  if ((qinfo->udata_len - qinfo->udata_offset) < data_len) {
@@ -4551,16 +4555,9 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
  QETH_CARD_TEXT_(card, 4, "sseqn%i",
  cmd->data.setadapterparms.hdr.seq_no);
  /*copy entries to user buffer*/
- if (cmd->data.setadapterparms.hdr.seq_no == 1) {
- memcpy(qinfo->udata + qinfo->udata_offset,
-       (char *)snmp,
-       data_len + offsetof(struct qeth_snmp_cmd, data));
- qinfo->udata_offset += offsetof(struct qeth_snmp_cmd, data);
- } else {
- memcpy(qinfo->udata + qinfo->udata_offset,
-       (char *)&snmp->request, data_len);
- }
+ memcpy(qinfo->udata + qinfo->udata_offset, snmp_data, data_len);
  qinfo->udata_offset += data_len;
+
  /* check if all replies received ... */
  QETH_CARD_TEXT_(card, 4, "srtot%i",
        cmd->data.setadapterparms.hdr.used_total);
--
2.7.4


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Cosmic][Disco][PATCH 1/1] s390/qeth: fix length check in SNMP processing

Kleber Souza
On 12/14/18 5:48 PM, Joseph Salisbury wrote:

> From: Julian Wiedmann <[hidden email]>
>
> BugLink: https://bugs.launchpad.net/bugs/1805802
>
> The response for a SNMP request can consist of multiple parts, which
> the cmd callback stages into a kernel buffer until all parts have been
> received. If the callback detects that the staging buffer provides
> insufficient space, it bails out with error.
> This processing is buggy for the first part of the response - while it
> initially checks for a length of 'data_len', it later copies an
> additional amount of 'offsetof(struct qeth_snmp_cmd, data)' bytes.
>
> Fix the calculation of 'data_len' for the first part of the response.
> This also nicely cleans up the memcpy code.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Julian Wiedmann <[hidden email]>
> Reviewed-by: Ursula Braun <[hidden email]>
> Signed-off-by: David S. Miller <[hidden email]>
> (cherry picked from commit 9a764c1e59684c0358e16ccaafd870629f2cfe67)
> Signed-off-by: Joseph Salisbury <[hidden email]>

Clean cherry-pick, limited to s390x:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>

> ---
>  drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
> index b828345..b043620 100644
> --- a/drivers/s390/net/qeth_core_main.c
> +++ b/drivers/s390/net/qeth_core_main.c
> @@ -4513,8 +4513,8 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
>  {
>   struct qeth_ipa_cmd *cmd;
>   struct qeth_arp_query_info *qinfo;
> - struct qeth_snmp_cmd *snmp;
>   unsigned char *data;
> + void *snmp_data;
>   __u16 data_len;
>  
>   QETH_CARD_TEXT(card, 3, "snpcmdcb");
> @@ -4522,7 +4522,6 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
>   cmd = (struct qeth_ipa_cmd *) sdata;
>   data = (unsigned char *)((char *)cmd - reply->offset);
>   qinfo = (struct qeth_arp_query_info *) reply->param;
> - snmp = &cmd->data.setadapterparms.data.snmp;
>  
>   if (cmd->hdr.return_code) {
>   QETH_CARD_TEXT_(card, 4, "scer1%x", cmd->hdr.return_code);
> @@ -4535,10 +4534,15 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
>   return 0;
>   }
>   data_len = *((__u16 *)QETH_IPA_PDU_LEN_PDU1(data));
> - if (cmd->data.setadapterparms.hdr.seq_no == 1)
> - data_len -= (__u16)((char *)&snmp->data - (char *)cmd);
> - else
> - data_len -= (__u16)((char *)&snmp->request - (char *)cmd);
> + if (cmd->data.setadapterparms.hdr.seq_no == 1) {
> + snmp_data = &cmd->data.setadapterparms.data.snmp;
> + data_len -= offsetof(struct qeth_ipa_cmd,
> +     data.setadapterparms.data.snmp);
> + } else {
> + snmp_data = &cmd->data.setadapterparms.data.snmp.request;
> + data_len -= offsetof(struct qeth_ipa_cmd,
> +     data.setadapterparms.data.snmp.request);
> + }
>  
>   /* check if there is enough room in userspace */
>   if ((qinfo->udata_len - qinfo->udata_offset) < data_len) {
> @@ -4551,16 +4555,9 @@ static int qeth_snmp_command_cb(struct qeth_card *card,
>   QETH_CARD_TEXT_(card, 4, "sseqn%i",
>   cmd->data.setadapterparms.hdr.seq_no);
>   /*copy entries to user buffer*/
> - if (cmd->data.setadapterparms.hdr.seq_no == 1) {
> - memcpy(qinfo->udata + qinfo->udata_offset,
> -       (char *)snmp,
> -       data_len + offsetof(struct qeth_snmp_cmd, data));
> - qinfo->udata_offset += offsetof(struct qeth_snmp_cmd, data);
> - } else {
> - memcpy(qinfo->udata + qinfo->udata_offset,
> -       (char *)&snmp->request, data_len);
> - }
> + memcpy(qinfo->udata + qinfo->udata_offset, snmp_data, data_len);
>   qinfo->udata_offset += data_len;
> +
>   /* check if all replies received ... */
>   QETH_CARD_TEXT_(card, 4, "srtot%i",
>         cmd->data.setadapterparms.hdr.used_total);



--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

Khaled Elmously
In reply to this post by Joseph Salisbury-3
On 2018-12-14 11:48:17 , Joseph Salisbury wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805802
>
> == SRU Justification ==
> The response for a SNMP request can consist of multiple parts,
>               which the cmd callback stages into a kernel buffer until all
>               parts have been received. If the callback detects that the
>               staging buffer provides insufficient space, it bails out with
>               error.
>               This processing is buggy for the first part of the response -
>               while it initially checks for a length of 'data_len', it later
>               copies an additional amount of
>               'offsetof(struct qeth_snmp_cmd, data)' bytes.
>
>
> == Fix ==
> 9a764c1e5968 ("s390/qeth: fix length check in SNMP processing")
>
> == Regression Potential ==
> Low.  Changes limited to s390.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Julian Wiedmann (1):
>   s390/qeth: fix length check in SNMP processing
>
>  drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>

Acked-by: Khalid Elmously <[hidden email]>
 

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[B, C]: [SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

Khaled Elmously
In reply to this post by Joseph Salisbury-3
On 2018-12-14 11:48:17 , Joseph Salisbury wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805802
>
> == SRU Justification ==
> The response for a SNMP request can consist of multiple parts,
>               which the cmd callback stages into a kernel buffer until all
>               parts have been received. If the callback detects that the
>               staging buffer provides insufficient space, it bails out with
>               error.
>               This processing is buggy for the first part of the response -
>               while it initially checks for a length of 'data_len', it later
>               copies an additional amount of
>               'offsetof(struct qeth_snmp_cmd, data)' bytes.
>
>
> == Fix ==
> 9a764c1e5968 ("s390/qeth: fix length check in SNMP processing")
>
> == Regression Potential ==
> Low.  Changes limited to s390.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.
>
> Julian Wiedmann (1):
>   s390/qeth: fix length check in SNMP processing
>
>  drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
>  1 file changed, 12 insertions(+), 15 deletions(-)
>
> --
> 2.7.4
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: APPLIED[B, C]: [SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

Stefan Bader-2
On 20.12.18 08:37, Khaled Elmously wrote:
> On 2018-12-14 11:48:17 , Joseph Salisbury wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1805802

The bug report does not reflect the applied state.

>>
>> == SRU Justification ==
>> The response for a SNMP request can consist of multiple parts,
>>               which the cmd callback stages into a kernel buffer until all
>>               parts have been received. If the callback detects that the
>>               staging buffer provides insufficient space, it bails out with
>>               error.
>>               This processing is buggy for the first part of the response -
>>               while it initially checks for a length of 'data_len', it later
>>               copies an additional amount of
>>               'offsetof(struct qeth_snmp_cmd, data)' bytes.
>>
>>
>> == Fix ==
>> 9a764c1e5968 ("s390/qeth: fix length check in SNMP processing")
>>
>> == Regression Potential ==
>> Low.  Changes limited to s390.
>>
>> == Test Case ==
>> A test kernel was built with this patch and tested by the original bug reporter.
>> The bug reporter states the test kernel resolved the bug.
>>
>> Julian Wiedmann (1):
>>   s390/qeth: fix length check in SNMP processing
>>
>>  drivers/s390/net/qeth_core_main.c | 27 ++++++++++++---------------
>>  1 file changed, 12 insertions(+), 15 deletions(-)
>>
>> --
>> 2.7.4
>>
>>
>> --
>> kernel-team mailing list
>> [hidden email]
>> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

NAK[D]: [SRU][Bionic][Cosmic][Disco][PATCH 0/1] s390/qeth: fix length check in SNMP processing

Seth Forshee
In reply to this post by Joseph Salisbury-3
On Fri, Dec 14, 2018 at 11:48:17AM -0500, Joseph Salisbury wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805802
>
> == SRU Justification ==
> The response for a SNMP request can consist of multiple parts,
>               which the cmd callback stages into a kernel buffer until all
>               parts have been received. If the callback detects that the
>               staging buffer provides insufficient space, it bails out with
>               error.
>               This processing is buggy for the first part of the response -
>               while it initially checks for a length of 'data_len', it later
>               copies an additional amount of
>               'offsetof(struct qeth_snmp_cmd, data)' bytes.
>
>
> == Fix ==
> 9a764c1e5968 ("s390/qeth: fix length check in SNMP processing")
>
> == Regression Potential ==
> Low.  Changes limited to s390.
>
> == Test Case ==
> A test kernel was built with this patch and tested by the original bug reporter.
> The bug reporter states the test kernel resolved the bug.

Not needed in disco as it has already come in from upstream stable.
Thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team