linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
@ 2020-08-21 16:52 Markus Mayer
  2020-08-22 11:56 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Mayer @ 2020-08-21 16:52 UTC (permalink / raw)
  To: Florian Fainelli, Colin Ian King, Krzysztof Kozlowski
  Cc: Markus Mayer, BCM Kernel Feedback, Linux ARM Kernel, Linux Kernel

We would overrun the error_text array if we hit a TIMEOUT condition,
because we were using the error code "ETIMEDOUT" (which is 110) as an
array index.

We fix the problem by correcting the array index and by providing a
function to retrieve error messages rather than accessing the array
directly. The function includes a bounds check that prevents the array
from being overrun.

This patch was prepared in response to
    https://lkml.org/lkml/2020/8/18/505.

Signed-off-by: Markus Mayer <mmayer@broadcom.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---

Changes since v1:
    - Added link of the coverity report to the commit message.
    - Added Florian's ack.
    - Removed second "const" from get_error_text() return type
      (thanks to the kernel test robot).

 drivers/memory/brcmstb_dpfe.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/memory/brcmstb_dpfe.c b/drivers/memory/brcmstb_dpfe.c
index e08528b12cbd..dcf50bb8dd69 100644
--- a/drivers/memory/brcmstb_dpfe.c
+++ b/drivers/memory/brcmstb_dpfe.c
@@ -188,11 +188,6 @@ struct brcmstb_dpfe_priv {
 	struct mutex lock;
 };
 
-static const char * const error_text[] = {
-	"Success", "Header code incorrect", "Unknown command or argument",
-	"Incorrect checksum", "Malformed command", "Timed out",
-};
-
 /*
  * Forward declaration of our sysfs attribute functions, so we can declare the
  * attribute data structures early.
@@ -307,6 +302,20 @@ static const struct dpfe_api dpfe_api_v3 = {
 	},
 };
 
+static const char *get_error_text(unsigned int i)
+{
+	static const char * const error_text[] = {
+		"Success", "Header code incorrect",
+		"Unknown command or argument", "Incorrect checksum",
+		"Malformed command", "Timed out", "Unknown error",
+	};
+
+	if (unlikely(i >= ARRAY_SIZE(error_text)))
+		i = ARRAY_SIZE(error_text) - 1;
+
+	return error_text[i];
+}
+
 static bool is_dcpu_enabled(struct brcmstb_dpfe_priv *priv)
 {
 	u32 val;
@@ -445,7 +454,7 @@ static int __send_command(struct brcmstb_dpfe_priv *priv, unsigned int cmd,
 	}
 	if (resp != 0) {
 		mutex_unlock(&priv->lock);
-		return -ETIMEDOUT;
+		return -ffs(DCPU_RET_ERR_TIMEDOUT);
 	}
 
 	/* Compute checksum over the message */
@@ -695,7 +704,7 @@ static ssize_t generic_show(unsigned int command, u32 response[],
 
 	ret = __send_command(priv, command, response);
 	if (ret < 0)
-		return sprintf(buf, "ERROR: %s\n", error_text[-ret]);
+		return sprintf(buf, "ERROR: %s\n", get_error_text(-ret));
 
 	return 0;
 }
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-21 16:52 [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds Markus Mayer
@ 2020-08-22 11:56 ` Krzysztof Kozlowski
  2020-08-22 16:40   ` Markus Mayer
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2020-08-22 11:56 UTC (permalink / raw)
  To: Markus Mayer
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> We would overrun the error_text array if we hit a TIMEOUT condition,
> because we were using the error code "ETIMEDOUT" (which is 110) as an
> array index.
> 
> We fix the problem by correcting the array index and by providing a
> function to retrieve error messages rather than accessing the array
> directly. The function includes a bounds check that prevents the array
> from being overrun.
> 
> This patch was prepared in response to
>     https://lkml.org/lkml/2020/8/18/505.
> 
> Signed-off-by: Markus Mayer <mmayer@broadcom.com>

Your Signed-off-by does not match From field. Please run
scripts/checkpatch on every patch you send.

I fixed it up, assuming markus.mayer@broadcom.com is the valid email
address.

> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> 
> Changes since v1:
>     - Added link of the coverity report to the commit message.
>     - Added Florian's ack.
>     - Removed second "const" from get_error_text() return type

Florian was so kind to provide you with necessary tags - Fixes and
Reported-by. Always include them on resubmit of patches.

Thanks, applied.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 11:56 ` Krzysztof Kozlowski
@ 2020-08-22 16:40   ` Markus Mayer
  2020-08-22 16:46     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Mayer @ 2020-08-22 16:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > We would overrun the error_text array if we hit a TIMEOUT condition,
> > because we were using the error code "ETIMEDOUT" (which is 110) as an
> > array index.
> >
> > We fix the problem by correcting the array index and by providing a
> > function to retrieve error messages rather than accessing the array
> > directly. The function includes a bounds check that prevents the array
> > from being overrun.
> >
> > This patch was prepared in response to
> >     https://lkml.org/lkml/2020/8/18/505.
> >
> > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>
> Your Signed-off-by does not match From field. Please run
> scripts/checkpatch on every patch you send.
>
> I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> address.

No. I have always been using mmayer@broadcom.com since it is shorter.
That's also what's in the MAINTAINERS file. Please change it back. I
accidentally used the long form for one of my e-mail replies which is
where the confusion must have originated.

> > Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> >
> > Changes since v1:
> >     - Added link of the coverity report to the commit message.
> >     - Added Florian's ack.
> >     - Removed second "const" from get_error_text() return type
>
> Florian was so kind to provide you with necessary tags - Fixes and
> Reported-by. Always include them on resubmit of patches.

I missed those. Thanks for catching it.

Regards,
-Markus



> Thanks, applied.
>
> Best regards,
> Krzysztof
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 16:40   ` Markus Mayer
@ 2020-08-22 16:46     ` Krzysztof Kozlowski
  2020-08-22 20:14       ` Markus Mayer
  0 siblings, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2020-08-22 16:46 UTC (permalink / raw)
  To: Markus Mayer
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >
> > On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > > We would overrun the error_text array if we hit a TIMEOUT condition,
> > > because we were using the error code "ETIMEDOUT" (which is 110) as an
> > > array index.
> > >
> > > We fix the problem by correcting the array index and by providing a
> > > function to retrieve error messages rather than accessing the array
> > > directly. The function includes a bounds check that prevents the array
> > > from being overrun.
> > >
> > > This patch was prepared in response to
> > >     https://lkml.org/lkml/2020/8/18/505.
> > >
> > > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >
> > Your Signed-off-by does not match From field. Please run
> > scripts/checkpatch on every patch you send.
> >
> > I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> > address.
> 
> No. I have always been using mmayer@broadcom.com since it is shorter.
> That's also what's in the MAINTAINERS file. Please change it back. I
> accidentally used the long form for one of my e-mail replies which is
> where the confusion must have originated.

I'll drop the patch then. You need to resend with SoB matching email.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 16:46     ` Krzysztof Kozlowski
@ 2020-08-22 20:14       ` Markus Mayer
  2020-08-22 20:21         ` Florian Fainelli
  0 siblings, 1 reply; 10+ messages in thread
From: Markus Mayer @ 2020-08-22 20:14 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> > On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > >
> > > On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > > > We would overrun the error_text array if we hit a TIMEOUT condition,
> > > > because we were using the error code "ETIMEDOUT" (which is 110) as an
> > > > array index.
> > > >
> > > > We fix the problem by correcting the array index and by providing a
> > > > function to retrieve error messages rather than accessing the array
> > > > directly. The function includes a bounds check that prevents the array
> > > > from being overrun.
> > > >
> > > > This patch was prepared in response to
> > > >     https://lkml.org/lkml/2020/8/18/505.
> > > >
> > > > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> > >
> > > Your Signed-off-by does not match From field. Please run
> > > scripts/checkpatch on every patch you send.
> > >
> > > I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> > > address.
> >
> > No. I have always been using mmayer@broadcom.com since it is shorter.
> > That's also what's in the MAINTAINERS file. Please change it back. I
> > accidentally used the long form for one of my e-mail replies which is
> > where the confusion must have originated.
>
> I'll drop the patch then. You need to resend with SoB matching email.

Oh, I am starting to see what's happening here. This is new and
apparently due to some changes with the mail server setup on our end.

I have this in my patch file:

$ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
From: Markus Mayer <mmayer@broadcom.com>
Date: Fri, 21 Aug 2020 08:56:52 -0700

Sending patches like this used to work. Clearly our SMTP server has
now taken it upon itself to rewrite the sender e-mail address. I
wasn't expecting that. Let me look into it. Sorry for the hassle. It
was not intentional.

Regards,
-Markus

> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 20:14       ` Markus Mayer
@ 2020-08-22 20:21         ` Florian Fainelli
  2020-08-22 20:46           ` Krzysztof Kozlowski
  2020-08-22 20:47           ` Markus Mayer
  0 siblings, 2 replies; 10+ messages in thread
From: Florian Fainelli @ 2020-08-22 20:21 UTC (permalink / raw)
  To: Markus Mayer, Krzysztof Kozlowski
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel



On 8/22/2020 1:14 PM, Markus Mayer wrote:
> On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
>>> On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
>>>>> We would overrun the error_text array if we hit a TIMEOUT condition,
>>>>> because we were using the error code "ETIMEDOUT" (which is 110) as an
>>>>> array index.
>>>>>
>>>>> We fix the problem by correcting the array index and by providing a
>>>>> function to retrieve error messages rather than accessing the array
>>>>> directly. The function includes a bounds check that prevents the array
>>>>> from being overrun.
>>>>>
>>>>> This patch was prepared in response to
>>>>>      https://lkml.org/lkml/2020/8/18/505.
>>>>>
>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>
>>>> Your Signed-off-by does not match From field. Please run
>>>> scripts/checkpatch on every patch you send.
>>>>
>>>> I fixed it up, assuming markus.mayer@broadcom.com is the valid email
>>>> address.
>>>
>>> No. I have always been using mmayer@broadcom.com since it is shorter.
>>> That's also what's in the MAINTAINERS file. Please change it back. I
>>> accidentally used the long form for one of my e-mail replies which is
>>> where the confusion must have originated.
>>
>> I'll drop the patch then. You need to resend with SoB matching email.
> 
> Oh, I am starting to see what's happening here. This is new and
> apparently due to some changes with the mail server setup on our end.
> 
> I have this in my patch file:
> 
> $ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
>  From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
> From: Markus Mayer <mmayer@broadcom.com>
> Date: Fri, 21 Aug 2020 08:56:52 -0700
> 
> Sending patches like this used to work. Clearly our SMTP server has
> now taken it upon itself to rewrite the sender e-mail address. I
> wasn't expecting that. Let me look into it. Sorry for the hassle. It
> was not intentional.

Yes, if you used to use the SMTP relay server which did not require 
authentication for internal hosts, and now you use smtp.gmail.com with 
your broadcom.com username, the SMTP server will rewrite the From: to 
match the username used to authenticate with the server.
-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 20:21         ` Florian Fainelli
@ 2020-08-22 20:46           ` Krzysztof Kozlowski
  2020-08-22 20:49             ` Markus Mayer
  2020-08-22 20:47           ` Markus Mayer
  1 sibling, 1 reply; 10+ messages in thread
From: Krzysztof Kozlowski @ 2020-08-22 20:46 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Markus Mayer, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, Aug 22, 2020 at 01:21:47PM -0700, Florian Fainelli wrote:
> 
> 
> On 8/22/2020 1:14 PM, Markus Mayer wrote:
> > On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > 
> > > On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> > > > On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > > 
> > > > > On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > > > > > We would overrun the error_text array if we hit a TIMEOUT condition,
> > > > > > because we were using the error code "ETIMEDOUT" (which is 110) as an
> > > > > > array index.
> > > > > > 
> > > > > > We fix the problem by correcting the array index and by providing a
> > > > > > function to retrieve error messages rather than accessing the array
> > > > > > directly. The function includes a bounds check that prevents the array
> > > > > > from being overrun.
> > > > > > 
> > > > > > This patch was prepared in response to
> > > > > >      https://lkml.org/lkml/2020/8/18/505.
> > > > > > 
> > > > > > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> > > > > 
> > > > > Your Signed-off-by does not match From field. Please run
> > > > > scripts/checkpatch on every patch you send.
> > > > > 
> > > > > I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> > > > > address.
> > > > 
> > > > No. I have always been using mmayer@broadcom.com since it is shorter.
> > > > That's also what's in the MAINTAINERS file. Please change it back. I
> > > > accidentally used the long form for one of my e-mail replies which is
> > > > where the confusion must have originated.
> > > 
> > > I'll drop the patch then. You need to resend with SoB matching email.
> > 
> > Oh, I am starting to see what's happening here. This is new and
> > apparently due to some changes with the mail server setup on our end.
> > 
> > I have this in my patch file:
> > 
> > $ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
> >  From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
> > From: Markus Mayer <mmayer@broadcom.com>
> > Date: Fri, 21 Aug 2020 08:56:52 -0700

Which means your patch actually passed checkpatch on your computer so my
comment about running it was not smart... :)

> > Sending patches like this used to work. Clearly our SMTP server has
> > now taken it upon itself to rewrite the sender e-mail address. I
> > wasn't expecting that. Let me look into it. Sorry for the hassle. It
> > was not intentional.
> 
> Yes, if you used to use the SMTP relay server which did not require
> authentication for internal hosts, and now you use smtp.gmail.com with your
> broadcom.com username, the SMTP server will rewrite the From: to match the
> username used to authenticate with the server.

Markus' patch did not go via GMail.  It was the Broadcom server which
mangled the things.  The email headers:

  Received: by mail.kernel.org (Postfix)
  Received: from rnd-relay.smtp.broadcom.com (rnd-relay.smtp.broadcom.com [192.19.229.170])
          by mail.kernel.org (Postfix) with ESMTPS id D5B3B20702
  Received: from mail-irv-17.broadcom.com (mail-irv-17.lvn.broadcom.net [10.75.242.48])
  Received: from lbrmn-mmayer.ric.broadcom.net (lbrmn-mmayer.ric.broadcom.net [10.136.28.150])
  From: Markus Mayer <markus.mayer@broadcom.com>

Neither kernel.org nor my final server (Gmail) cares about usernames in
From fields of some specific domain.

I guess you could try configure you git send email to use email of
"markus.mayer" while keeping Author "mmayer".  You have to configure git
send email for this and such configuration results in proper two "From"
fields which maybe smtp broadcom won't change.

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 20:21         ` Florian Fainelli
  2020-08-22 20:46           ` Krzysztof Kozlowski
@ 2020-08-22 20:47           ` Markus Mayer
  2020-08-22 20:49             ` Florian Fainelli
  1 sibling, 1 reply; 10+ messages in thread
From: Markus Mayer @ 2020-08-22 20:47 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Krzysztof Kozlowski, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, 22 Aug 2020 at 13:21, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> On 8/22/2020 1:14 PM, Markus Mayer wrote:
> > On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> >>> On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>>>
> >>>> On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> >>>>> We would overrun the error_text array if we hit a TIMEOUT condition,
> >>>>> because we were using the error code "ETIMEDOUT" (which is 110) as an
> >>>>> array index.
> >>>>>
> >>>>> We fix the problem by correcting the array index and by providing a
> >>>>> function to retrieve error messages rather than accessing the array
> >>>>> directly. The function includes a bounds check that prevents the array
> >>>>> from being overrun.
> >>>>>
> >>>>> This patch was prepared in response to
> >>>>>      https://lkml.org/lkml/2020/8/18/505.
> >>>>>
> >>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> >>>>
> >>>> Your Signed-off-by does not match From field. Please run
> >>>> scripts/checkpatch on every patch you send.
> >>>>
> >>>> I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> >>>> address.
> >>>
> >>> No. I have always been using mmayer@broadcom.com since it is shorter.
> >>> That's also what's in the MAINTAINERS file. Please change it back. I
> >>> accidentally used the long form for one of my e-mail replies which is
> >>> where the confusion must have originated.
> >>
> >> I'll drop the patch then. You need to resend with SoB matching email.
> >
> > Oh, I am starting to see what's happening here. This is new and
> > apparently due to some changes with the mail server setup on our end.
> >
> > I have this in my patch file:
> >
> > $ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
> >  From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
> > From: Markus Mayer <mmayer@broadcom.com>
> > Date: Fri, 21 Aug 2020 08:56:52 -0700
> >
> > Sending patches like this used to work. Clearly our SMTP server has
> > now taken it upon itself to rewrite the sender e-mail address. I
> > wasn't expecting that. Let me look into it. Sorry for the hassle. It
> > was not intentional.
>
> Yes, if you used to use the SMTP relay server which did not require
> authentication for internal hosts, and now you use smtp.gmail.com with
> your broadcom.com username, the SMTP server will rewrite the From: to
> match the username used to authenticate with the server.

Actually, it was the other way around. Connecting to smtp.gmail.com
does allow the "From:" header to be customized. The envelope sender,
i.e. the "From " line at the very beginning of the e-mail, might still
get rewritten, but that's okay since the "From:" header is left alone.
The internal SMTP server, however, which does not require
authentication, unexpectedly rewrites the "From:" header in the middle
of the e-mail header.

Got it set up now in a way that should work. At least it did in my
test. I'll send out v3 of the patch momentarily, and then we'll know
for sure.

Regards,
-Markus

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 20:46           ` Krzysztof Kozlowski
@ 2020-08-22 20:49             ` Markus Mayer
  0 siblings, 0 replies; 10+ messages in thread
From: Markus Mayer @ 2020-08-22 20:49 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Florian Fainelli, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel

On Sat, 22 Aug 2020 at 13:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sat, Aug 22, 2020 at 01:21:47PM -0700, Florian Fainelli wrote:
> >
> >
> > On 8/22/2020 1:14 PM, Markus Mayer wrote:
> > > On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > >
> > > > On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
> > > > > On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > > > > >
> > > > > > On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
> > > > > > > We would overrun the error_text array if we hit a TIMEOUT condition,
> > > > > > > because we were using the error code "ETIMEDOUT" (which is 110) as an
> > > > > > > array index.
> > > > > > >
> > > > > > > We fix the problem by correcting the array index and by providing a
> > > > > > > function to retrieve error messages rather than accessing the array
> > > > > > > directly. The function includes a bounds check that prevents the array
> > > > > > > from being overrun.
> > > > > > >
> > > > > > > This patch was prepared in response to
> > > > > > >      https://lkml.org/lkml/2020/8/18/505.
> > > > > > >
> > > > > > > Signed-off-by: Markus Mayer <mmayer@broadcom.com>
> > > > > >
> > > > > > Your Signed-off-by does not match From field. Please run
> > > > > > scripts/checkpatch on every patch you send.
> > > > > >
> > > > > > I fixed it up, assuming markus.mayer@broadcom.com is the valid email
> > > > > > address.
> > > > >
> > > > > No. I have always been using mmayer@broadcom.com since it is shorter.
> > > > > That's also what's in the MAINTAINERS file. Please change it back. I
> > > > > accidentally used the long form for one of my e-mail replies which is
> > > > > where the confusion must have originated.
> > > >
> > > > I'll drop the patch then. You need to resend with SoB matching email.
> > >
> > > Oh, I am starting to see what's happening here. This is new and
> > > apparently due to some changes with the mail server setup on our end.
> > >
> > > I have this in my patch file:
> > >
> > > $ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
> > >  From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
> > > From: Markus Mayer <mmayer@broadcom.com>
> > > Date: Fri, 21 Aug 2020 08:56:52 -0700
>
> Which means your patch actually passed checkpatch on your computer so my
> comment about running it was not smart... :)

It did pass, yes. :-)

> > > Sending patches like this used to work. Clearly our SMTP server has
> > > now taken it upon itself to rewrite the sender e-mail address. I
> > > wasn't expecting that. Let me look into it. Sorry for the hassle. It
> > > was not intentional.
> >
> > Yes, if you used to use the SMTP relay server which did not require
> > authentication for internal hosts, and now you use smtp.gmail.com with your
> > broadcom.com username, the SMTP server will rewrite the From: to match the
> > username used to authenticate with the server.
>
> Markus' patch did not go via GMail.  It was the Broadcom server which
> mangled the things.  The email headers:

Indeed.

>   Received: by mail.kernel.org (Postfix)
>   Received: from rnd-relay.smtp.broadcom.com (rnd-relay.smtp.broadcom.com [192.19.229.170])
>           by mail.kernel.org (Postfix) with ESMTPS id D5B3B20702
>   Received: from mail-irv-17.broadcom.com (mail-irv-17.lvn.broadcom.net [10.75.242.48])
>   Received: from lbrmn-mmayer.ric.broadcom.net (lbrmn-mmayer.ric.broadcom.net [10.136.28.150])
>   From: Markus Mayer <markus.mayer@broadcom.com>
>
> Neither kernel.org nor my final server (Gmail) cares about usernames in
> From fields of some specific domain.
>
> I guess you could try configure you git send email to use email of
> "markus.mayer" while keeping Author "mmayer".  You have to configure git
> send email for this and such configuration results in proper two "From"
> fields which maybe smtp broadcom won't change.

Yep. I think I've got it set up now. Stay tuned.

> Best regards,
> Krzysztof

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds
  2020-08-22 20:47           ` Markus Mayer
@ 2020-08-22 20:49             ` Florian Fainelli
  0 siblings, 0 replies; 10+ messages in thread
From: Florian Fainelli @ 2020-08-22 20:49 UTC (permalink / raw)
  To: Markus Mayer
  Cc: Krzysztof Kozlowski, Colin Ian King, BCM Kernel Feedback,
	Linux ARM Kernel, Linux Kernel



On 8/22/2020 1:47 PM, Markus Mayer wrote:
> On Sat, 22 Aug 2020 at 13:21, Florian Fainelli <f.fainelli@gmail.com> wrote:
>>
>> On 8/22/2020 1:14 PM, Markus Mayer wrote:
>>> On Sat, 22 Aug 2020 at 09:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On Sat, Aug 22, 2020 at 09:40:59AM -0700, Markus Mayer wrote:
>>>>> On Sat, 22 Aug 2020 at 04:56, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>
>>>>>> On Fri, Aug 21, 2020 at 09:52:21AM -0700, Markus Mayer wrote:
>>>>>>> We would overrun the error_text array if we hit a TIMEOUT condition,
>>>>>>> because we were using the error code "ETIMEDOUT" (which is 110) as an
>>>>>>> array index.
>>>>>>>
>>>>>>> We fix the problem by correcting the array index and by providing a
>>>>>>> function to retrieve error messages rather than accessing the array
>>>>>>> directly. The function includes a bounds check that prevents the array
>>>>>>> from being overrun.
>>>>>>>
>>>>>>> This patch was prepared in response to
>>>>>>>       https://lkml.org/lkml/2020/8/18/505.
>>>>>>>
>>>>>>> Signed-off-by: Markus Mayer <mmayer@broadcom.com>
>>>>>>
>>>>>> Your Signed-off-by does not match From field. Please run
>>>>>> scripts/checkpatch on every patch you send.
>>>>>>
>>>>>> I fixed it up, assuming markus.mayer@broadcom.com is the valid email
>>>>>> address.
>>>>>
>>>>> No. I have always been using mmayer@broadcom.com since it is shorter.
>>>>> That's also what's in the MAINTAINERS file. Please change it back. I
>>>>> accidentally used the long form for one of my e-mail replies which is
>>>>> where the confusion must have originated.
>>>>
>>>> I'll drop the patch then. You need to resend with SoB matching email.
>>>
>>> Oh, I am starting to see what's happening here. This is new and
>>> apparently due to some changes with the mail server setup on our end.
>>>
>>> I have this in my patch file:
>>>
>>> $ head 0001-memory-brcmstb_dpfe-fix-array-index-out-of-bounds.patch
>>>   From 6b424772d4c84fa56474b2522d0d3ed6b2b2b360 Mon Sep 17 00:00:00 2001
>>> From: Markus Mayer <mmayer@broadcom.com>
>>> Date: Fri, 21 Aug 2020 08:56:52 -0700
>>>
>>> Sending patches like this used to work. Clearly our SMTP server has
>>> now taken it upon itself to rewrite the sender e-mail address. I
>>> wasn't expecting that. Let me look into it. Sorry for the hassle. It
>>> was not intentional.
>>
>> Yes, if you used to use the SMTP relay server which did not require
>> authentication for internal hosts, and now you use smtp.gmail.com with
>> your broadcom.com username, the SMTP server will rewrite the From: to
>> match the username used to authenticate with the server.
> 
> Actually, it was the other way around. Connecting to smtp.gmail.com
> does allow the "From:" header to be customized. The envelope sender,
> i.e. the "From " line at the very beginning of the e-mail, might still
> get rewritten, but that's okay since the "From:" header is left alone.
> The internal SMTP server, however, which does not require
> authentication, unexpectedly rewrites the "From:" header in the middle
> of the e-mail header.
> 
> Got it set up now in a way that should work. At least it did in my
> test. I'll send out v3 of the patch momentarily, and then we'll know
> for sure.

Reason #42 why I hide behind my gmail.com account! Glad you sorted it out.
-- 
Florian

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-08-22 20:49 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-21 16:52 [PATCH v2] memory: brcmstb_dpfe: fix array index out of bounds Markus Mayer
2020-08-22 11:56 ` Krzysztof Kozlowski
2020-08-22 16:40   ` Markus Mayer
2020-08-22 16:46     ` Krzysztof Kozlowski
2020-08-22 20:14       ` Markus Mayer
2020-08-22 20:21         ` Florian Fainelli
2020-08-22 20:46           ` Krzysztof Kozlowski
2020-08-22 20:49             ` Markus Mayer
2020-08-22 20:47           ` Markus Mayer
2020-08-22 20:49             ` Florian Fainelli

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).