On 12/28/21 12:56, Uwe Kleine-König wrote:
On Mon, Dec 27, 2021 at 05:02:39PM +0200, Jarkko Nikula wrote:
On 12/27/21 11:45, Uwe Kleine-König wrote:
This fixes device lifetime issues where it was possible to free a live
struct device.
Fixes: b711f687a1c1 ("counter: Add support for Intel Quadrature Encoder Peripheral")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
drivers/counter/intel-qep.c | 28 ++++++++++++++++------------
1 file changed, 16 insertions(+), 12 deletions(-)
Should the Fixes tag rather be b6c50affda59 ("counter: Add character device
interface") instead of when each drivers were introduced? I mean was it
possible to hit the issue before /dev/counter was introduced?
I'm not sure if there is an issue before this, but it was already wrong
before for sure. Maybe it's possible to hold a reference somehow via
sysfs?
The thought that made me tag the individual driver commits was: With the
approach used to fix the issue all drivers need to be modified and an
unconverted driver doesn't benefit from the availability of
counter_alloc() / counter_add() if it isn't used. So all trees that
include b711f687a1c1 but not "counter: intel-qep: Convert to new counter
registration" are broken (more or less exploitable). So I think the
added Fixes line is the right choice.
Fair enough. Noticed only now after this patch (with patches 1, 4, 8 and
13 applied) second counter instance initialization fails:
[ 8.113999] sysfs: cannot create duplicate filename '/dev/char/244:0
[ 8.139604] Call Trace:
[ 8.139606] <TASK>
[ 8.139608] show_stack+0x3d/0x3f
[ 8.139615] dump_stack_lvl+0x5b/0x82
[ 8.139619] dump_stack+0x10/0x12
[ 8.139621] sysfs_warn_dup.cold+0x17/0x27
[ 8.139624] sysfs_do_create_link_sd+0xc2/0xd0
[ 8.139629] sysfs_create_link+0x1c/0x30
[ 8.139631] device_add+0x54f/0x7c0
[ 8.139635] ? cdev_default_release+0x20/0x20
[ 8.139638] cdev_device_add+0x47/0x90
[ 8.139642] devm_counter_add+0x61/0xe0 [counter]
[ 8.139647] intel_qep_probe+0x16f/0x1b0
...
[ 8.325081] intel-qep 0000:00:18.6: error -EEXIST: Failed to add counter
[ 8.371793] intel-qep: probe of 0000:00:18.6 failed with error -17
Positive result is that following test doesn't produce an oops after a
few iterations like before this patch.
while :; do { sleep 5; echo bang; } > /dev/counter0 & sleep 1; echo
0000:00:18.4 >/sys/bus/pci/drivers/intel-qep/unbind; sleep 8; echo
0000:00:18.4 >/sys/bus/pci/drivers/intel-qep/bind; done
