Re: svn commit: r284336 - head/sys/dev/acpi_support
On Sat, Jun 13, 2015 at 03:31:16PM -0700, Garrett Cooper wrote: > On Jun 13, 2015, at 4:30, Marcelo Araujo wrote: > > > +1. > > Fixed (r284357/r284358). Thanks! I doubt that anything is fixed. >From a little information I can gather from the r284336 commit message, there was some NULL pointer dereference (may be not, FWIW). The r284336 papered over it, just ignoring the chunk of code when bith env variables are _not_ NULL. Now, the condition is reversed, and since r284336 hide the bug, it is arguable that both env vars were not NULL on the problematic machines. The consequence is that the check is a nop now, since machines do have valid SMBIOS info, and a bug, whatever it is, is not fixed, and probably not even diagnosed. ___ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284336 - head/sys/dev/acpi_support
On Jun 13, 2015, at 4:30, Marcelo Araujo wrote: > +1. Fixed (r284357/r284358). Thanks! signature.asc Description: Message signed with OpenPGP using GPGMail
Re: svn commit: r284336 - head/sys/dev/acpi_support
+1.
On Jun 13, 2015 3:42 PM, "NGie Cooper" wrote:
> On Sat, Jun 13, 2015 at 12:37 AM, Konstantin Belousov
> wrote:
> > On Sat, Jun 13, 2015 at 05:55:26AM +, Allan Jude wrote:
> >> Author: allanjude (doc committer)
> >> Date: Sat Jun 13 05:55:26 2015
> >> New Revision: 284336
> >> URL: https://svnweb.freebsd.org/changeset/base/284336
> >>
> >> Log:
> >> acpi_ibm.ko panics if SMBIOS information is not available
> >>
> >> Add a check for NULL before strcmp on smbios information incase it is
> not populated
> >>
> >> Differential Revision: https://reviews.freebsd.org/D2750
> >> Reviewed by:ngie, jhb
> >> Approved by:rpaulo
> >> Sponsored by: ScaleEngine Inc.
> >>
> >> Modified:
> >> head/sys/dev/acpi_support/acpi_ibm.c
> >>
> >> Modified: head/sys/dev/acpi_support/acpi_ibm.c
> >>
> ==
> >> --- head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 01:28:19 2015
> (r284335)
> >> +++ head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 05:55:26 2015
> (r284336)
> >> @@ -485,6 +485,9 @@ acpi_ibm_attach(device_t dev)
> >> /* Enable per-model events. */
> >> maker = kern_getenv("smbios.system.maker");
> >> product = kern_getenv("smbios.system.product");
> >> + if (maker != NULL && product != NULL)
> >> + goto nosmbios;
> > This looks reversed. I would expect the condition to be
> > if (maker == NULL || product == NULL)
> > goto ...;
>
> +1
> ___
> svn-src-h...@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284336 - head/sys/dev/acpi_support
On Sat, Jun 13, 2015 at 12:37 AM, Konstantin Belousov
wrote:
> On Sat, Jun 13, 2015 at 05:55:26AM +, Allan Jude wrote:
>> Author: allanjude (doc committer)
>> Date: Sat Jun 13 05:55:26 2015
>> New Revision: 284336
>> URL: https://svnweb.freebsd.org/changeset/base/284336
>>
>> Log:
>> acpi_ibm.ko panics if SMBIOS information is not available
>>
>> Add a check for NULL before strcmp on smbios information incase it is not
>> populated
>>
>> Differential Revision: https://reviews.freebsd.org/D2750
>> Reviewed by:ngie, jhb
>> Approved by:rpaulo
>> Sponsored by: ScaleEngine Inc.
>>
>> Modified:
>> head/sys/dev/acpi_support/acpi_ibm.c
>>
>> Modified: head/sys/dev/acpi_support/acpi_ibm.c
>> ==
>> --- head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 01:28:19 2015
>> (r284335)
>> +++ head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 05:55:26 2015
>> (r284336)
>> @@ -485,6 +485,9 @@ acpi_ibm_attach(device_t dev)
>> /* Enable per-model events. */
>> maker = kern_getenv("smbios.system.maker");
>> product = kern_getenv("smbios.system.product");
>> + if (maker != NULL && product != NULL)
>> + goto nosmbios;
> This looks reversed. I would expect the condition to be
> if (maker == NULL || product == NULL)
> goto ...;
+1
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
Re: svn commit: r284336 - head/sys/dev/acpi_support
On Sat, Jun 13, 2015 at 05:55:26AM +, Allan Jude wrote:
> Author: allanjude (doc committer)
> Date: Sat Jun 13 05:55:26 2015
> New Revision: 284336
> URL: https://svnweb.freebsd.org/changeset/base/284336
>
> Log:
> acpi_ibm.ko panics if SMBIOS information is not available
>
> Add a check for NULL before strcmp on smbios information incase it is not
> populated
>
> Differential Revision: https://reviews.freebsd.org/D2750
> Reviewed by:ngie, jhb
> Approved by:rpaulo
> Sponsored by: ScaleEngine Inc.
>
> Modified:
> head/sys/dev/acpi_support/acpi_ibm.c
>
> Modified: head/sys/dev/acpi_support/acpi_ibm.c
> ==
> --- head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 01:28:19 2015
> (r284335)
> +++ head/sys/dev/acpi_support/acpi_ibm.c Sat Jun 13 05:55:26 2015
> (r284336)
> @@ -485,6 +485,9 @@ acpi_ibm_attach(device_t dev)
> /* Enable per-model events. */
> maker = kern_getenv("smbios.system.maker");
> product = kern_getenv("smbios.system.product");
> + if (maker != NULL && product != NULL)
> + goto nosmbios;
This looks reversed. I would expect the condition to be
if (maker == NULL || product == NULL)
goto ...;
> +
> for (i = 0; i < nitems(acpi_ibm_models); i++) {
> if (strcmp(maker, acpi_ibm_models[i].maker) == 0 &&
> strcmp(product, acpi_ibm_models[i].product) == 0) {
> @@ -494,6 +497,8 @@ acpi_ibm_attach(device_t dev)
> ACPI_SERIAL_END(ibm);
> }
> }
> +
> +nosmbios:
> freeenv(maker);
> freeenv(product);
>
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
