Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201218040826.57203-1-decui@microsoft.com> <MW2PR2101MB1052E61BEB80F14896EEFA79D7DF9@MW2PR2101MB1052.namprd21.prod.outlook.com>
 <MW2PR2101MB181943C255A86A11E5402E50BFD19@MW2PR2101MB1819.namprd21.prod.outlook.com>
In-Reply-To: <MW2PR2101MB181943C255A86A11E5402E50BFD19@MW2PR2101MB1819.namprd21.prod.outlook.com>
From:   "Rafael J. Wysocki" <rafael@kernel.org>
Date:   Thu, 7 Jan 2021 14:47:34 +0100
Message-ID: <CAJZ5v0gXW0oEmduxZEc0BZWxfQd=+jh_r88OZAHmwCW=GFNBGg@mail.gmail.com>
Subject: Re: [PATCH] ACPI: scan: Fix a Hyper-V Linux VM panic caused by buffer overflow
To:     Dexuan Cui <decui@microsoft.com>
Cc:     Michael Kelley <mikelley@microsoft.com>,
        "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "rjw@rjwysocki.net" <rjw@rjwysocki.net>,
        "len.brown@intel.com" <len.brown@intel.com>,
        "wei.liu@kernel.org" <wei.liu@kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Stephen Hemminger <sthemmin@microsoft.com>,
        Haiyang Zhang <haiyangz@microsoft.com>,
        KY Srinivasan <kys@microsoft.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, Jan 5, 2021 at 11:02 PM Dexuan Cui <decui@microsoft.com> wrote:
>
> > From: Michael Kelley <mikelley@microsoft.com>
> > Sent: Tuesday, December 22, 2020 5:56 AM
> > From: Dexuan Cui
> > Sent: Thursday, December 17, 2020
> > 8:08 PM
> > >
> > > Linux VM on Hyper-V crashes with the latest mainline:
> > > ...
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -674,7 +674,8 @@ int acpi_device_add(struct acpi_device *device,
> > >     }
> > >     if (!found) {
> > >             acpi_device_bus_id = new_bus_id;
> > > -           strcpy(acpi_device_bus_id->bus_id, acpi_device_hid(device));
> > > +           strlcpy(acpi_device_bus_id->bus_id, acpi_device_hid(device),
> > > +                   sizeof(acpi_device_bus_id->bus_id));
> > >             acpi_device_bus_id->instance_no = 0;
> > >             list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
> > >     }
> >
> > Reviewed-by: Michael Kelley <mikelley@microsoft.com>
>
> Hi, ACPI maintainers,
> Would you please take a look at the small fix? Currently the mainline Linux
> kernel, running in a VM on Hyper-V, has been broken for almost 3 weeks,
> i.e. the VM always panics when it boots.

The root cause is a VM issue AFAICS, though.

> The patch has already had Michael's Reviewed-by.
>
> BTW, the patch should have a stable tag:
> Cc: <stable@vger.kernel.org>
>
> Or, do you want the patch to go through the Hyper-V tree?
> https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git/log/?h=hyperv-fixes
>
> The small patch is unlikely to cause a merge conflict, and it only affects
> Linux VMs on Hyper-V so far.

It doesn't look like the right fix to me, though.

The problem appears to be that the string coming from _HID is too long
(which is a spec violation).  The patch truncates it to match the
length of the target buffer, but that is not particularly useful.

It would be better to use something like kstrdup_const() to initialize
acpi_device_bus_id->bus_id IMV.