lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Sun, 4 Aug 2013 01:42:49 -0500
From:	Felipe Contreras <felipe.contreras@...il.com>
To:	Aaron Lu <aaron.lwe@...il.com>
Cc:	"Rafael J. Wysocki" <rjw@...k.pl>, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, Len Brown <lenb@...nel.org>,
	Zhang Rui <rui.zhang@...el.com>
Subject: Re: [PATCH] acpi: video: improve quirk check

On Sat, Aug 3, 2013 at 8:18 PM, Aaron Lu <aaron.lwe@...il.com> wrote:
> On 08/03/2013 07:34 PM, Rafael J. Wysocki wrote:
>> On Saturday, August 03, 2013 04:14:04 PM Aaron Lu wrote:
>>> On 08/03/2013 07:47 AM, Rafael J. Wysocki wrote:
>>>> On Friday, August 02, 2013 02:37:09 PM Felipe Contreras wrote:
>>>>> If the _BCL package is descending, the first level (br->levels[2]) will
>>>>> be 0, and if the number of levels matches the number of steps, we might
>>>>> confuse a returned level to mean the index.
>>>>>
>>>>> For example:
>>>>>
>>>>>   current_level = max_level = 100
>>>>>   test_level = 0
>>>>>   returned level = 100
>>>>>
>>>>> In this case 100 means the level, not the index, and _BCM failed. But if
>>>>> the _BCL package is descending, the index of level 0 is also 100, so we
>>>>> assume _BQC is indexed, when it's not.
>>>>>
>>>>> This causes all _BQC calls to return bogus values causing weird behavior
>>>>> from the user's perspective. For example: xbacklight -set 10; xbacklight
>>>>> -set 20; would flash to 90% and then slowly down to the desired level
>>>>> (20).
>>>>>
>>>>> The solution is simple; test anything other than the first level (e.g.
>>>>> 1).
>>>>>
>>>>> Signed-off-by: Felipe Contreras <felipe.contreras@...il.com>
>>>>
>>>> Looks reasonable.
>>>>
>>>> Aaron, what do you think?
>>>
>>> Yes, the patch is correct, but I still prefer my own version :-)
>>> https://github.com/aaronlu/linux/commit/0a3d2c5b59caf80ae5bb1ca1fda0f7bf448b38c9
>>>
>>> In case you want to take mine and mine needs refresh, please let me know
>>> and I can do the re-base, thanks.
>>
>> Well, I prefer simpler, unless there's a good reason to use more complicated.
>>
>> Why exactly do you think your version is better?
>
> As explained here:
> https://lkml.org/lkml/2013/8/2/81
> https://lkml.org/lkml/2013/8/2/112
>
> And for the demo broken _BQC, mine patch will disable _BQC while still
> make the backlight work, and this patch here is testing the max
> brightness level and may fail.

Yes, but that problem can *only* happen in such a simplified _BCL,
which is very very unlikely. Still, it would make sense to fix the
code for that case.

However, we can fix the problem first for the real known cases with my
simple one-liner patch that can even be merged for v3.11, and *later*
fix the issue for the synthetic unlikely case.

Personally I think there are better ways to fix the code for the
synthetic case than what you patch does, which will also make _BQC
work. That can be discussed later though, the one-liner is simple, and
it works.

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists