linux-kernel - Re: [PATCH] soundwire: intel_auxdevice: Don't disable IRQs before removing children

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: <9b8ef4d2-e4bd-4336-8980-0fb59d672631@linux.dev>
Date: Wed, 18 Dec 2024 16:40:22 -0500
From: Pierre-Louis Bossart <pierre-louis.bossart@...ux.dev>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: Richard Fitzgerald <rf@...nsource.cirrus.com>, vkoul@...nel.org,
 peter.ujfalusi@...ux.intel.com, yung-chuan.liao@...ux.intel.com,
 sanyog.r.kale@...el.com, linux-sound@...r.kernel.org,
 linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH] soundwire: intel_auxdevice: Don't disable IRQs before
 removing children


>> Or do you object to
>> the way I have solved that problem? I am certainly open to other
>> solutions, if you have any suggestions?

Having looked at the code in more details, I think there are other 
issues, see e.g. this part of the code called from snd_bus_master_delete().

static int sdw_delete_slave(struct device *dev, void *data)
{
	struct sdw_slave *slave = dev_to_sdw_dev(dev);
	struct sdw_bus *bus = slave->bus;

	pm_runtime_disable(dev);

	sdw_slave_debugfs_exit(slave);

	mutex_lock(&bus->bus_lock);

	if (slave->dev_num) { /* clear dev_num if assigned */
		clear_bit(slave->dev_num, bus->assigned);
		if (bus->ops && bus->ops->put_device_num)
			bus->ops->put_device_num(bus, slave);
	}

So at this point an interaction with the device is not longer possible, 
even if the Cadence interrupts are kept active, since there's no valid 
device number to use...

	list_del_init(&slave->node);
	mutex_unlock(&bus->bus_lock);

... but this is where the .remove() will take place.

	device_unregister(dev);
	return 0;
}

What am I missing?