linux-kernel - Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight sysfs uevent

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]

Message-Id: <20131215235336.49d49a25.akpm@linux-foundation.org>
Date:	Sun, 15 Dec 2013 23:53:36 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Henrique de Moraes Holschuh <hmh@....eng.br>
Cc:	Matthew Garrett <mjg59@...f.ucam.org>, linux-fbdev@...r.kernel.org,
	"'Kyungmin Park'" <kmpark@...radead.org>, kay@...y.org,
	Jingoo Han <jg1.han@...sung.com>,
	"'Henrique de Moraes Holschuh'" <ibm-acpi@....eng.br>,
	linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
	ibm-acpi-devel@...ts.sourceforge.net,
	"'Richard Purdie'" <rpurdie@...ys.net>
Subject: Re: [ibm-acpi-devel] [PATCH] video: backlight: Remove backlight
 sysfs uevent

On Sun, 24 Nov 2013 01:53:11 -0200 Henrique de Moraes Holschuh <hmh@....eng.br> wrote:

> On Sun, 24 Nov 2013, Matthew Garrett wrote:
> > On Sat, Nov 23, 2013 at 10:40:15PM -0200, Henrique de Moraes Holschuh wrote:
> > > On Fri, 22 Nov 2013, Matthew Garrett wrote:
> > > > We have userspace that relies on uevents of type 
> > > > BACKLIGHT_UPDATE_HOTKEY. I don't know that we have userspace that relies 
> > > > on uevents of type BACKLIGHT_UPDATE_SYSFS.
> > > 
> > > Any OSD application would have to rely on both uevent types, or it is broken
> > > (and to test that, just write a level to sysfs and watch the OSD app fail to
> > > tell you about the backlight level change...)
> > 
> > Right, OSDs are supposed to respond to keypresses, not arbitrary changes 
> > of backlight. If the user's just echoed 8 into brightness, they know 
> > they set the brightness to 8 - they don't need an OSD to tell them that. 
> 
> It is not just the user that sets the brightness.
> 
> Still, if you're sure that all userspace users react only to the hotkey type
> of event, removing the sysfs one won't break anything any further.
> 
> But it will be *really* annoying the day we revisit this because someone
> started abusing the hotkey uevent and we have to deploy a proper fix (rate
> limiting or switching to a proper event report interface that doesn't use
> uevents).
> 
> > BACKLIGHT_UPDATE_HOTKEY is when the firmware itself has changed the 
> > brightness in response to a keypress, and so reporting the keypress 
> > would result in additional backlight changes.
> 
> Yeah, I know that bug quite well, thinkpads were the first victims of
> idiotic feedback event loops caused by braindead userspace.

I'm not seeing a lot of consensus here and afaict the v2 patch:

--- a/drivers/video/backlight/backlight.c~drivers-video-backlight-backlightc-remove-backlight-sysfs-uevent
+++ a/drivers/video/backlight/backlight.c
@@ -175,8 +175,6 @@ static ssize_t brightness_store(struct d
 	}
 	mutex_unlock(&bd->ops_lock);
 
-	backlight_generate_event(bd, BACKLIGHT_UPDATE_SYSFS);
-
 	return rc;
 }
 static DEVICE_ATTR_RW(brightness);

will still break userspace which relies on BACKLIGHT_UPDATE_SYSFS
uevents.  I see no way we can guarantee that there is no such userspace
so the patch is worrying.

Should we instead be looking for a way of avoiding this risk?  Say, add
a new knob which people can set if they don't want to generate this
event?  Ugly, but that's the price we pay for mucking it up originally.
--
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/