Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1735109

Re: [PATCH v3] platform/chrome: Use proper protocol transfer function

From Brian Norris <briannorris@chromium.org>
Newsgroups linux.kernel
Subject Re: [PATCH v3] platform/chrome: Use proper protocol transfer function
Date 2017-09-19 19:20 +0200
Message-ID <urulI-8fP-1@gated-at.bofh.it> (permalink)
References <unyxA-3Kx-9@gated-at.bofh.it> <urr4u-66a-23@gated-at.bofh.it> <urrxw-6uO-9@gated-at.bofh.it> <urtSG-7OZ-7@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi Jon,

On Tue, Sep 19, 2017 at 05:39:56PM +0100, Jon Hunter wrote:
> On 19/09/17 15:09, Shawn N wrote:
> > On Tue, Sep 19, 2017 at 6:44 AM, Jon Hunter <jonathanh@nvidia.com> wrote:
> >> Tegra124 Nyan-Big is currently crashing during boot with -next [0] and
> >> bisect is pointing to this commit. Reverting the above on top of -next
> >> does allow the board to boot successfully. Looks like this board is
> >> proto_version 3 but I have not looked into this any further. Let me know
> >> if you have any thoughts.
> > 
> > 
> > Thanks for the bug report, I'll look into this today.

Yes, thanks!

> >> [    1.502497] kernel BUG at drivers/platform/chrome/cros_ec_proto.c:34!
> >> 34 BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
> > 
> > So, ec_dev->proto_version > 3? That doesn't seem right.
> 
> You mean != 3, but yes. Looks like an initialisation problem, because if I 
> add the following WARNING ...
> 
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index e7bbdf947bbc..ad3b3a1e8d54 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -31,6 +31,7 @@ static int prepare_packet(struct cros_ec_device *ec_dev,
>         int i;
>         u8 csum = 0;
>  
> +       WARN(ec_dev->proto_version != EC_HOST_REQUEST_VERSION, "%d != %d", ec_dev->proto_version, EC_HOST_REQUEST_VERSION);
>         BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
>         BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
>  
> ... then I see ...
> 
> [    1.502495] WARNING: CPU: 0 PID: 1 at drivers/platform/chrome/cros_ec_proto.c:35 cros_ec_prepare_tx+0x190/0x1a8
> [    1.512566] 65535 != 3
> 
> Any chance this is being called before the version is initialised?

If it's uninitialized, it should be 0 (the structure is kzalloc'd, and
the call stack you point to clearly shows it's at least been allocated
already). Also, if it's uninitialized, then you should be BUG'ing even
without this patch; the patch you've bisected to is only modifying the
*second* (or later) attempt to send the command, and it's using the same
'ec_dev' structure.

Furthermore, the only assignments to this 'proto_version' field look
like they're only writing one of 0, 2, 3, or

   min(EC_HOST_REQUEST_VERSION, fls(proto_info->protocol_versions) - 1)

. I don't see where 0xffff comes from.

So...is there any chance we've got a heap corruption somewhere?
Somebody's overwriting 'ec_dev->proto_version' accidentally?

Brian

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Jon Hunter <jonathanh@nvidia.com> - 2017-09-19 15:50 +0200
  Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Shawn N <shawnn@chromium.org> - 2017-09-19 16:20 +0200
    Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Jon Hunter <jonathanh@nvidia.com> - 2017-09-19 18:50 +0200
      Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Shawn N <shawnn@chromium.org> - 2017-09-19 19:10 +0200
      Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Brian Norris <briannorris@chromium.org> - 2017-09-19 19:20 +0200
        Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Shawn N <shawnn@chromium.org> - 2017-09-20 08:10 +0200
          Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Brian Norris <briannorris@chromium.org> - 2017-09-20 08:20 +0200
            Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Shawn N <shawnn@google.com> - 2017-09-20 22:30 +0200
              Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Shawn N <shawnn@chromium.org> - 2017-09-26 01:20 +0200
                Re: [PATCH v3] platform/chrome: Use proper protocol transfer function Jon Hunter <jonathanh@nvidia.com> - 2017-09-26 17:50 +0200

csiph-web