| Message ID | 1442408287-10410-1-git-send-email-sudeep.holla@arm.com |
|---|---|
| State | New |
| Headers | show |
On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: > acpi_parse_entries passes the table end pointer to the sub-table entry > handler. acpi_parse_entries itself could validate the end of an entry > against the table end using the length in the sub-table entry. > > This patch adds the validation of the sub-table entry end using the > length field.This will help to eliminate the need to pass the table end > to the handlers. > > It also moves the check for zero length entry early so that execution of > the handler can be avoided. > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > > --- > drivers/acpi/tables.c | 31 +++++++++++++++---------------- > 1 file changed, 15 insertions(+), 16 deletions(-) > > Hi Rafael, > > As I mentioned earlier, this needs to be applied after Al's MADT changes > are merged. You might get simple conflicts in acpi_parse_entries. This needs to be rebased on top of some patches in my linux-next branch. It probably is better to rebase it on top of my bleeding-edge branch that contains the Al's patches already, though. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 26/09/15 01:27, Rafael J. Wysocki wrote: > On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: >> acpi_parse_entries passes the table end pointer to the sub-table entry >> handler. acpi_parse_entries itself could validate the end of an entry >> against the table end using the length in the sub-table entry. >> >> This patch adds the validation of the sub-table entry end using the >> length field.This will help to eliminate the need to pass the table end >> to the handlers. >> >> It also moves the check for zero length entry early so that execution of >> the handler can be avoided. >> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >> >> --- >> drivers/acpi/tables.c | 31 +++++++++++++++---------------- >> 1 file changed, 15 insertions(+), 16 deletions(-) >> >> Hi Rafael, >> >> As I mentioned earlier, this needs to be applied after Al's MADT changes >> are merged. You might get simple conflicts in acpi_parse_entries. > > This needs to be rebased on top of some patches in my linux-next branch. > > It probably is better to rebase it on top of my bleeding-edge branch that > contains the Al's patches already, though. > I don't see Al's patches in your linux-next or bleeding-edge Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 28/09/15 14:50, Rafael J. Wysocki wrote: > On Monday, September 28, 2015 11:11:11 AM Sudeep Holla wrote: >> >> On 26/09/15 01:27, Rafael J. Wysocki wrote: >>> On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: >>>> acpi_parse_entries passes the table end pointer to the sub-table entry >>>> handler. acpi_parse_entries itself could validate the end of an entry >>>> against the table end using the length in the sub-table entry. >>>> >>>> This patch adds the validation of the sub-table entry end using the >>>> length field.This will help to eliminate the need to pass the table end >>>> to the handlers. >>>> >>>> It also moves the check for zero length entry early so that execution of >>>> the handler can be avoided. >>>> >>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>> >>>> --- >>>> drivers/acpi/tables.c | 31 +++++++++++++++---------------- >>>> 1 file changed, 15 insertions(+), 16 deletions(-) >>>> >>>> Hi Rafael, >>>> >>>> As I mentioned earlier, this needs to be applied after Al's MADT changes >>>> are merged. You might get simple conflicts in acpi_parse_entries. >>> >>> This needs to be rebased on top of some patches in my linux-next branch. >>> >>> It probably is better to rebase it on top of my bleeding-edge branch that >>> contains the Al's patches already, though. >>> >> >> I don't see Al's patches in your linux-next or bleeding-edge > > They were there, but I've dropped them due to a 0-day testing failure. > Yes I guess we did see this last week, I had ask Al to fix it privately. It was some discrepancy with ACPIv1.0 specification between different sections that resulted in failures I saw. > I think your patches depend on the Al's ones, is that correct? > Correct, I think it's easier if I wait for his patches. Regards, Sudeep -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday, September 28, 2015 11:11:11 AM Sudeep Holla wrote: > > On 26/09/15 01:27, Rafael J. Wysocki wrote: > > On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: > >> acpi_parse_entries passes the table end pointer to the sub-table entry > >> handler. acpi_parse_entries itself could validate the end of an entry > >> against the table end using the length in the sub-table entry. > >> > >> This patch adds the validation of the sub-table entry end using the > >> length field.This will help to eliminate the need to pass the table end > >> to the handlers. > >> > >> It also moves the check for zero length entry early so that execution of > >> the handler can be avoided. > >> > >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > >> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> > >> > >> --- > >> drivers/acpi/tables.c | 31 +++++++++++++++---------------- > >> 1 file changed, 15 insertions(+), 16 deletions(-) > >> > >> Hi Rafael, > >> > >> As I mentioned earlier, this needs to be applied after Al's MADT changes > >> are merged. You might get simple conflicts in acpi_parse_entries. > > > > This needs to be rebased on top of some patches in my linux-next branch. > > > > It probably is better to rebase it on top of my bleeding-edge branch that > > contains the Al's patches already, though. > > > > I don't see Al's patches in your linux-next or bleeding-edge They were there, but I've dropped them due to a 0-day testing failure. I think your patches depend on the Al's ones, is that correct? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
On 09/28/2015 07:37 AM, Sudeep Holla wrote: > > > On 28/09/15 14:50, Rafael J. Wysocki wrote: >> On Monday, September 28, 2015 11:11:11 AM Sudeep Holla wrote: >>> >>> On 26/09/15 01:27, Rafael J. Wysocki wrote: >>>> On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: >>>>> acpi_parse_entries passes the table end pointer to the sub-table entry >>>>> handler. acpi_parse_entries itself could validate the end of an entry >>>>> against the table end using the length in the sub-table entry. >>>>> >>>>> This patch adds the validation of the sub-table entry end using the >>>>> length field.This will help to eliminate the need to pass the table end >>>>> to the handlers. >>>>> >>>>> It also moves the check for zero length entry early so that execution of >>>>> the handler can be avoided. >>>>> >>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>>> >>>>> --- >>>>> drivers/acpi/tables.c | 31 +++++++++++++++---------------- >>>>> 1 file changed, 15 insertions(+), 16 deletions(-) >>>>> >>>>> Hi Rafael, >>>>> >>>>> As I mentioned earlier, this needs to be applied after Al's MADT changes >>>>> are merged. You might get simple conflicts in acpi_parse_entries. >>>> >>>> This needs to be rebased on top of some patches in my linux-next branch. >>>> >>>> It probably is better to rebase it on top of my bleeding-edge branch that >>>> contains the Al's patches already, though. >>>> >>> >>> I don't see Al's patches in your linux-next or bleeding-edge >> >> They were there, but I've dropped them due to a 0-day testing failure. >> > > Yes I guess we did see this last week, I had ask Al to fix it privately. > It was some discrepancy with ACPIv1.0 specification between different > sections that resulted in failures I saw. > >> I think your patches depend on the Al's ones, is that correct? >> > > Correct, I think it's easier if I wait for his patches. > > Regards, > Sudeep My apologies. Was participating in family stuff all weekend and Linaro Connect all last week. This appears to be an incorrect reading of the 1.0 spec, and not being able to find the 1.0b version, on my part. Unfortunately, http://www.acpi.info/DOWNLOADS/*spec*.pdf is not public so one has to guess at files names for older versions of the spec -- and I assumed 1.0B, the current naming practice. Sorry about that...the patch is pretty simple, I think. Rafael, which tree do you want me to base the respin on? Your bleeding-edge branch?
Hi, On Mon, Sep 28, 2015 at 9:39 PM, Al Stone <al.stone@linaro.org> wrote: > On 09/28/2015 07:37 AM, Sudeep Holla wrote: >> >> >> On 28/09/15 14:50, Rafael J. Wysocki wrote: >>> On Monday, September 28, 2015 11:11:11 AM Sudeep Holla wrote: >>>> >>>> On 26/09/15 01:27, Rafael J. Wysocki wrote: >>>>> On Wednesday, September 16, 2015 01:58:06 PM Sudeep Holla wrote: >>>>>> acpi_parse_entries passes the table end pointer to the sub-table entry >>>>>> handler. acpi_parse_entries itself could validate the end of an entry >>>>>> against the table end using the length in the sub-table entry. >>>>>> >>>>>> This patch adds the validation of the sub-table entry end using the >>>>>> length field.This will help to eliminate the need to pass the table end >>>>>> to the handlers. >>>>>> >>>>>> It also moves the check for zero length entry early so that execution of >>>>>> the handler can be avoided. >>>>>> >>>>>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >>>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> >>>>>> >>>>>> --- >>>>>> drivers/acpi/tables.c | 31 +++++++++++++++---------------- >>>>>> 1 file changed, 15 insertions(+), 16 deletions(-) >>>>>> >>>>>> Hi Rafael, >>>>>> >>>>>> As I mentioned earlier, this needs to be applied after Al's MADT changes >>>>>> are merged. You might get simple conflicts in acpi_parse_entries. >>>>> >>>>> This needs to be rebased on top of some patches in my linux-next branch. >>>>> >>>>> It probably is better to rebase it on top of my bleeding-edge branch that >>>>> contains the Al's patches already, though. >>>>> >>>> >>>> I don't see Al's patches in your linux-next or bleeding-edge >>> >>> They were there, but I've dropped them due to a 0-day testing failure. >>> >> >> Yes I guess we did see this last week, I had ask Al to fix it privately. >> It was some discrepancy with ACPIv1.0 specification between different >> sections that resulted in failures I saw. >> >>> I think your patches depend on the Al's ones, is that correct? >>> >> >> Correct, I think it's easier if I wait for his patches. >> >> Regards, >> Sudeep > > My apologies. Was participating in family stuff all weekend > and Linaro Connect all last week. > > This appears to be an incorrect reading of the 1.0 spec, and not > being able to find the 1.0b version, on my part. Unfortunately, > http://www.acpi.info/DOWNLOADS/*spec*.pdf is not public so one has > to guess at files names for older versions of the spec -- and I > assumed 1.0B, the current naming practice. > > Sorry about that...the patch is pretty simple, I think. Rafael, > which tree do you want me to base the respin on? Your bleeding-edge > branch? My linux-next branch should be OK for that. I guess there will be a conflict between your patches and the Marc's ones, but I can resolve that one I suppose. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c index d0716bb6359d..69b9d05f5b96 100644 --- a/drivers/acpi/tables.c +++ b/drivers/acpi/tables.c @@ -459,7 +459,7 @@ acpi_parse_entries(char *id, unsigned long table_size, { struct acpi_subtable_header *entry; int count = 0; - unsigned long table_end; + unsigned long table_end, entry_end; if (acpi_disabled) return -ENODEV; @@ -478,12 +478,20 @@ acpi_parse_entries(char *id, unsigned long table_size, table_end = (unsigned long)table_header + table_header->length; /* Parse all entries looking for a match. */ + entry_end = (unsigned long)table_header + table_size; + entry = (struct acpi_subtable_header *)entry_end; + entry_end += entry->length; - entry = (struct acpi_subtable_header *) - ((unsigned long)table_header + table_size); + while (entry_end <= table_end) { + /* + * If entry->length is 0, break from this loop to avoid + * infinite loop. + */ + if (entry->length == 0) { + pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, entry_id); + return -EINVAL; + } - while (((unsigned long)entry) + sizeof(struct acpi_subtable_header) < - table_end) { if (entry->type == entry_id && (!max_entries || count < max_entries)) { if (!strncmp(id, ACPI_SIG_MADT, 4) && @@ -495,17 +503,8 @@ acpi_parse_entries(char *id, unsigned long table_size, count++; } - /* - * If entry->length is 0, break from this loop to avoid - * infinite loop. - */ - if (entry->length == 0) { - pr_err("[%4.4s:0x%02x] Invalid zero length\n", id, entry_id); - return -EINVAL; - } - - entry = (struct acpi_subtable_header *) - ((unsigned long)entry + entry->length); + entry = (struct acpi_subtable_header *)entry_end; + entry_end += entry->length; } if (max_entries && count > max_entries) {
acpi_parse_entries passes the table end pointer to the sub-table entry handler. acpi_parse_entries itself could validate the end of an entry against the table end using the length in the sub-table entry. This patch adds the validation of the sub-table entry end using the length field.This will help to eliminate the need to pass the table end to the handlers. It also moves the check for zero length entry early so that execution of the handler can be avoided. Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com> --- drivers/acpi/tables.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) Hi Rafael, As I mentioned earlier, this needs to be applied after Al's MADT changes are merged. You might get simple conflicts in acpi_parse_entries. Regards, Sudeep v1->v2: - Incorporated Rafael's review comments - Moved zero length entry check early - Added a patch to remove the unused table_end parameter -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/