2022-12-08 18:32:08

by Bartosz Golaszewski

[permalink] [raw]
Subject: [PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel

From: Bartosz Golaszewski <[email protected]>

If we open an i2c character device and then unbind the underlying i2c
adapter (either by unbinding it manually via sysfs or - for a real-life
example - when unplugging a USB device with an i2c adaper), the kernel
thread calling i2c_del_adapter() will become blocked waiting for the
completion that only completes once all references to the character
device get dropped.

In order to fix that, we introduce a couple changes. They need to be
part of a single commit in order to preserve bisectability. First, drop
the dev_release completion. That removes the risk of a deadlock but
we now need to protect the character device structures against NULL
pointer dereferences. To that end introduce an rw semaphore. It will
protect the dummy i2c_client structure against dropping the adapter from
under it. It will be taken for reading by all file_operations callbacks
and for writing by the notifier's unbind handler. This way we don't
prohibit the syscalls that don't get in each other's way from running
concurrently but the adapter will not be unbound before all syscalls
return.

Finally: upon being notified about an unbind event for the i2c adapter,
we take the lock for writing and set the adapter pointer in the character
device's structure to NULL. This "numbs down" the device - it still exists
but is no longer functional. Meanwhile every syscall callback checks that
pointer after taking the lock but before executing any code that requires
it. If it's NULL, we return an error to user-space.

This way we can safely open an i2c device from user-space, unbind the
device without triggering a deadlock and any subsequent system-call for
the file descriptor associated with the removed adapter will gracefully
fail.

Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/i2c/i2c-core-base.c | 18 -------
drivers/i2c/i2c-dev.c | 96 ++++++++++++++++++++++++++++++++-----
include/linux/i2c.h | 2 -
3 files changed, 83 insertions(+), 33 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 7539b0740351..368fe271558f 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1137,12 +1137,6 @@ EXPORT_SYMBOL_GPL(i2c_new_ancillary_device);

/* I2C bus adapters -- one roots each I2C or SMBUS segment */

-static void i2c_adapter_dev_release(struct device *dev)
-{
- struct i2c_adapter *adap = to_i2c_adapter(dev);
- complete(&adap->dev_released);
-}
-
unsigned int i2c_adapter_depth(struct i2c_adapter *adapter)
{
unsigned int depth = 0;
@@ -1292,7 +1286,6 @@ ATTRIBUTE_GROUPS(i2c_adapter);

struct device_type i2c_adapter_type = {
.groups = i2c_adapter_groups,
- .release = i2c_adapter_dev_release,
};
EXPORT_SYMBOL_GPL(i2c_adapter_type);

@@ -1513,9 +1506,7 @@ static int i2c_register_adapter(struct i2c_adapter *adap)
return 0;

out_reg:
- init_completion(&adap->dev_released);
device_unregister(&adap->dev);
- wait_for_completion(&adap->dev_released);
out_list:
mutex_lock(&core_lock);
idr_remove(&i2c_adapter_idr, adap->nr);
@@ -1714,16 +1705,7 @@ void i2c_del_adapter(struct i2c_adapter *adap)

i2c_host_notify_irq_teardown(adap);

- /* wait until all references to the device are gone
- *
- * FIXME: This is old code and should ideally be replaced by an
- * alternative which results in decoupling the lifetime of the struct
- * device from the i2c_adapter, like spi or netdev do. Any solution
- * should be thoroughly tested with DEBUG_KOBJECT_RELEASE enabled!
- */
- init_completion(&adap->dev_released);
device_unregister(&adap->dev);
- wait_for_completion(&adap->dev_released);

/* free bus id */
mutex_lock(&core_lock);
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 107623c4cc14..305b64a5d4d4 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -28,6 +28,7 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/notifier.h>
+#include <linux/rwsem.h>
#include <linux/slab.h>
#include <linux/uaccess.h>

@@ -44,8 +45,14 @@ struct i2c_dev {
struct i2c_adapter *adap;
struct device dev;
struct cdev cdev;
+ struct rw_semaphore sem;
};

+static inline struct i2c_dev *to_i2c_dev(struct inode *ino)
+{
+ return container_of(ino->i_cdev, struct i2c_dev, cdev);
+}
+
#define I2C_MINORS (MINORMASK + 1)
static LIST_HEAD(i2c_dev_list);
static DEFINE_SPINLOCK(i2c_dev_list_lock);
@@ -136,15 +143,23 @@ static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count,
{
char *tmp;
int ret;
-
+ struct i2c_dev *i2c_dev = to_i2c_dev(file_inode(file));
struct i2c_client *client = file->private_data;

if (count > 8192)
count = 8192;

+ down_read(&i2c_dev->sem);
+ if (!i2c_dev->adap) {
+ up_read(&i2c_dev->sem);
+ return -ENODEV;
+ }
+
tmp = kzalloc(count, GFP_KERNEL);
- if (tmp == NULL)
+ if (tmp == NULL) {
+ up_read(&i2c_dev->sem);
return -ENOMEM;
+ }

pr_debug("i2c-%d reading %zu bytes.\n", iminor(file_inode(file)), count);

@@ -152,6 +167,7 @@ static ssize_t i2cdev_read(struct file *file, char __user *buf, size_t count,
if (ret >= 0)
if (copy_to_user(buf, tmp, ret))
ret = -EFAULT;
+ up_read(&i2c_dev->sem);
kfree(tmp);
return ret;
}
@@ -161,18 +177,28 @@ static ssize_t i2cdev_write(struct file *file, const char __user *buf,
{
int ret;
char *tmp;
+ struct i2c_dev *i2c_dev = to_i2c_dev(file_inode(file));
struct i2c_client *client = file->private_data;

if (count > 8192)
count = 8192;

+ down_read(&i2c_dev->sem);
+ if (!i2c_dev->adap) {
+ up_read(&i2c_dev->sem);
+ return -ENODEV;
+ }
+
tmp = memdup_user(buf, count);
- if (IS_ERR(tmp))
+ if (IS_ERR(tmp)) {
+ up_read(&i2c_dev->sem);
return PTR_ERR(tmp);
+ }

pr_debug("i2c-%d writing %zu bytes.\n", iminor(file_inode(file)), count);

ret = i2c_master_send(client, tmp, count);
+ up_read(&i2c_dev->sem);
kfree(tmp);
return ret;
}
@@ -389,7 +415,8 @@ static noinline int i2cdev_ioctl_smbus(struct i2c_client *client,
return res;
}

-static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long i2cdev_ioctl_unlocked(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct i2c_client *client = file->private_data;
unsigned long funcs;
@@ -495,6 +522,20 @@ static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
return 0;
}

+static long i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ struct i2c_dev *i2c_dev = to_i2c_dev(file_inode(file));
+ long ret;
+
+ down_read(&i2c_dev->sem);
+ if (!i2c_dev->adap)
+ ret = -ENODEV;
+ else
+ ret = i2cdev_ioctl_unlocked(file, cmd, arg);
+ up_read(&i2c_dev->sem);
+
+ return ret;
+}
#ifdef CONFIG_COMPAT

struct i2c_smbus_ioctl_data32 {
@@ -516,10 +557,12 @@ struct i2c_rdwr_ioctl_data32 {
u32 nmsgs;
};

-static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+static long compat_i2cdev_ioctl_unlocked(struct file *file, unsigned int cmd,
+ unsigned long arg)
{
struct i2c_client *client = file->private_data;
unsigned long funcs;
+
switch (cmd) {
case I2C_FUNCS:
funcs = i2c_get_functionality(client->adapter);
@@ -578,19 +621,39 @@ static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd, unsigned lo
return i2cdev_ioctl(file, cmd, arg);
}
}
+
+static long compat_i2cdev_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct i2c_dev *i2c_dev = to_i2c_dev(file_inode(file));
+ long ret;
+
+ down_read(&i2c_dev->sem);
+ if (!i2c_dev->adap)
+ ret = -ENODEV;
+ else
+ ret = compat_i2cdev_ioctl_unlocked(file, cmd, arg);
+ up_read(&i2c_dev->sem);
+
+ return ret;
+}
#else
#define compat_i2cdev_ioctl NULL
#endif

static int i2cdev_open(struct inode *inode, struct file *file)
{
- unsigned int minor = iminor(inode);
+ struct i2c_dev *i2c_dev = to_i2c_dev(inode);
struct i2c_client *client;
struct i2c_adapter *adap;
+ int ret = 0;

- adap = i2c_get_adapter(minor);
- if (!adap)
- return -ENODEV;
+ down_read(&i2c_dev->sem);
+ adap = i2c_dev->adap;
+ if (!adap) {
+ ret = -ENODEV;
+ goto out_unlock;
+ }

/* This creates an anonymous i2c_client, which may later be
* pointed to some address using I2C_SLAVE or I2C_SLAVE_FORCE.
@@ -601,22 +664,23 @@ static int i2cdev_open(struct inode *inode, struct file *file)
*/
client = kzalloc(sizeof(*client), GFP_KERNEL);
if (!client) {
- i2c_put_adapter(adap);
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto out_unlock;
}
snprintf(client->name, I2C_NAME_SIZE, "i2c-dev %d", adap->nr);

client->adapter = adap;
file->private_data = client;

- return 0;
+out_unlock:
+ up_read(&i2c_dev->sem);
+ return ret;
}

static int i2cdev_release(struct inode *inode, struct file *file)
{
struct i2c_client *client = file->private_data;

- i2c_put_adapter(client->adapter);
kfree(client);
file->private_data = NULL;

@@ -669,6 +733,8 @@ static int i2cdev_attach_adapter(struct device *dev, void *dummy)
i2c_dev->dev.parent = &adap->dev;
i2c_dev->dev.release = i2cdev_dev_release;

+ init_rwsem(&i2c_dev->sem);
+
res = dev_set_name(&i2c_dev->dev, "i2c-%d", adap->nr);
if (res)
goto err_put_i2c_dev;
@@ -698,6 +764,10 @@ static int i2cdev_detach_adapter(struct device *dev, void *dummy)
if (!i2c_dev) /* attach_adapter must have failed */
return NOTIFY_DONE;

+ down_write(&i2c_dev->sem);
+ i2c_dev->adap = NULL;
+ up_write(&i2c_dev->sem);
+
put_i2c_dev(i2c_dev, true);

pr_debug("adapter [%s] unregistered\n", adap->name);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index f7c49bbdb8a1..5c4adf921245 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -14,7 +14,6 @@
#include <linux/bits.h>
#include <linux/mod_devicetable.h>
#include <linux/device.h> /* for struct device */
-#include <linux/sched.h> /* for completion */
#include <linux/mutex.h>
#include <linux/regulator/consumer.h>
#include <linux/rtmutex.h>
@@ -738,7 +737,6 @@ struct i2c_adapter {

int nr;
char name[48];
- struct completion dev_released;

struct mutex userspace_clients_lock;
struct list_head userspace_clients;
--
2.37.2


2022-12-11 17:32:10

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel

Greeting,

FYI, we noticed WARNING:at_drivers/base/core.c:#device_release due to commit (built with gcc-11):

commit: 22f82cbe7636f59cc21f6837c500d704e3b412a5 ("[PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel")
url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/i2c-fortify-the-subsystem-against-user-space-induced-deadlocks/20221209-022450
base: https://git.kernel.org/cgit/linux/kernel/git/wsa/linux.git i2c/for-next
patch link: https://lore.kernel.org/all/[email protected]/
patch subject: [PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel

in testcase: xfstests
version: xfstests-x86_64-5a5e419-1_20221128
with following parameters:

disk: 4HDD
fs: xfs
test: xfs-group-07

test-description: xfstests is a regression test suite for xfs and other files ystems.
test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git

on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz (Ivy Bridge) with 8G memory

caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):


kern :warn : [ 35.378637] ------------[ cut here ]------------
kern :err : [ 35.388198] Device 'i2c-7' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.
kern :warn : [ 35.401052] WARNING: CPU: 1 PID: 195 at drivers/base/core.c:2332 device_release (drivers/base/core.c:2332)
kern :warn : [ 35.410178] Modules linked in: x86_pkg_temp_thermal intel_powerclamp i915(+) sd_mod t10_pi crc64_rocksoft_generic crc64_rocksoft ipmi_devintf crc64 coretemp ipmi_msghandler sg kvm_intel drm_buddy intel_gtt kvm irqbypass crct10dif_pclmul drm_display_helper crc32_pclmul ttm crc32c_intel ghash_clmulni_intel drm_kms_helper sha512_ssse3 syscopyarea ahci mei_me rapl intel_cstate libahci sysfillrect wmi_bmof sysimgblt i2c_i801 mei intel_uncore libata i2c_smbus lpc_ich fb_sys_fops video wmi drm fuse ip_tables
kern :warn : [ 35.455372] CPU: 1 PID: 195 Comm: systemd-udevd Not tainted 6.1.0-rc8-00063-g22f82cbe7636 #1
kern :warn : [ 35.464544] Hardware name: Hewlett-Packard HP Pro 3340 MT/17A1, BIOS 8.07 01/24/2013
kern :warn : [ 35.473036] RIP: 0010:device_release (drivers/base/core.c:2332)
kern :warn : [ 35.478405] Code: 48 8d 7d 50 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 89 00 00 00 48 8b 75 50 48 85 f6 74 13 48 c7 c7 60 f0 ce 83 e8 6c 1c f2 00 <0f> 0b e9 0f ff ff ff 48 b8 00 00 00 00 00 fc ff df 48 89 ea 48 c1
All code
========
0: 48 8d 7d 50 lea 0x50(%rbp),%rdi
4: 48 89 fa mov %rdi,%rdx
7: 48 c1 ea 03 shr $0x3,%rdx
b: 80 3c 02 00 cmpb $0x0,(%rdx,%rax,1)
f: 0f 85 89 00 00 00 jne 0x9e
15: 48 8b 75 50 mov 0x50(%rbp),%rsi
19: 48 85 f6 test %rsi,%rsi
1c: 74 13 je 0x31
1e: 48 c7 c7 60 f0 ce 83 mov $0xffffffff83cef060,%rdi
25: e8 6c 1c f2 00 callq 0xf21c96
2a:* 0f 0b ud2 <-- trapping instruction
2c: e9 0f ff ff ff jmpq 0xffffffffffffff40
31: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
38: fc ff df
3b: 48 89 ea mov %rbp,%rdx
3e: 48 rex.W
3f: c1 .byte 0xc1

Code starting with the faulting instruction
===========================================
0: 0f 0b ud2
2: e9 0f ff ff ff jmpq 0xffffffffffffff16
7: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
e: fc ff df
11: 48 89 ea mov %rbp,%rdx
14: 48 rex.W
15: c1 .byte 0xc1
kern :warn : [ 35.497913] RSP: 0018:ffffc900008af310 EFLAGS: 00010286
kern :warn : [ 35.503884] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
kern :debug : [ 35.507134] calling acpi_cpufreq_init+0x0/0xdeb [acpi_cpufreq] @ 190
kern :warn : [ 35.511739] RDX: 0000000000000004 RSI: 0000000000000008 RDI: fffff52000115e54
kern :debug : [ 35.518879] initcall acpi_cpufreq_init+0x0/0xdeb [acpi_cpufreq] returned -17 after 1 usecs
kern :warn : [ 35.526749] RBP: ffff88811b5671e8 R08: 0000000000000001 R09: ffffc900008af147
kern :warn : [ 35.526761] R10: fffff52000115e28 R11: 0000000000000001 R12: ffff888215592a00
kern :warn : [ 35.551527] R13: 0000000000000000 R14: ffff88811b5671e8 R15: ffff88811b567220
kern :warn : [ 35.559414] FS: 00007f9cb90ec8c0(0000) GS:ffff88818e880000(0000) knlGS:0000000000000000
kern :warn : [ 35.568257] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
kern :warn : [ 35.574756] CR2: 0000556c7b996000 CR3: 000000011d29a003 CR4: 00000000001706e0
kern :warn : [ 35.582650] Call Trace:
kern :warn : [ 35.585828] <TASK>
kern :warn : [ 35.588667] kobject_cleanup (lib/kobject.c:677)
kern :warn : [ 35.593256] i2c_del_adapter (drivers/i2c/i2c-core-base.c:1710)
kern :warn : [ 35.597944] ? drm_encoder_cleanup (include/linux/list.h:134 include/linux/list.h:148 drivers/gpu/drm/drm_encoder.c:199) drm
kern :warn : [ 35.603630] ? __process_removed_driver (drivers/i2c/i2c-core-base.c:1655)
kern :warn : [ 35.609083] ? drm_encoder_cleanup (include/linux/list.h:134 include/linux/list.h:148 drivers/gpu/drm/drm_encoder.c:199) drm
kern :warn : [ 35.614859] intel_sdvo_init (drivers/gpu/drm/i915/display/intel_sdvo.c:2626 drivers/gpu/drm/i915/display/intel_sdvo.c:3436) i915
kern :warn : [ 35.620500] ? intel_sdvo_get_hw_state (drivers/gpu/drm/i915/display/intel_sdvo.c:3313) i915
kern :warn : [ 35.626889] ? fwtable_read16 (drivers/gpu/drm/i915/intel_uncore.c:1851) i915
kern :warn : [ 35.632481] intel_setup_outputs (drivers/gpu/drm/i915/display/intel_display.c:8005) i915
kern :warn : [ 35.638516] intel_modeset_init_nogem (drivers/gpu/drm/i915/display/intel_display.c:8764) i915
kern :warn : [ 35.644817] i915_driver_probe (drivers/gpu/drm/i915/i915_driver.c:966) i915
kern :warn : [ 35.650560] ? __mutex_unlock_slowpath+0x2a0/0x2a0
kern :warn : [ 35.657141] ? i915_print_iommu_status (drivers/gpu/drm/i915/i915_driver.c:889) i915
kern :warn : [ 35.663336] ? drm_privacy_screen_get (drivers/gpu/drm/drm_privacy_screen.c:167) drm
kern :warn : [ 35.669388] i915_pci_probe (drivers/gpu/drm/i915/i915_pci.c:1337) i915
kern :warn : [ 35.674966] ? i915_pci_remove (drivers/gpu/drm/i915/i915_pci.c:1305) i915
kern :warn : [ 35.680594] ? _raw_read_unlock_irqrestore (kernel/locking/spinlock.c:161)
kern :warn : [ 35.686339] ? __cond_resched (kernel/sched/core.c:8343)
kern :warn : [ 35.690950] ? i915_pci_remove (drivers/gpu/drm/i915/i915_pci.c:1305) i915
kern :warn : [ 35.696432] local_pci_probe (drivers/pci/pci-driver.c:324)
kern :warn : [ 35.701038] pci_call_probe (drivers/pci/pci-driver.c:392)
kern :warn : [ 35.705638] ? _raw_spin_lock (arch/x86/include/asm/atomic.h:202 include/linux/atomic/atomic-instrumented.h:543 include/asm-generic/qspinlock.h:111 include/linux/spinlock.h:186 include/linux/spinlock_api_smp.h:134 kernel/locking/spinlock.c:154)
kern :warn : [ 35.710260] ? pci_pm_suspend_noirq (drivers/pci/pci-driver.c:352)
kern :warn : [ 35.715549] ? pci_assign_irq (drivers/pci/setup-irq.c:25)
kern :warn : [ 35.720242] ? pci_match_device (drivers/pci/pci-driver.c:108 drivers/pci/pci-driver.c:159)
kern :warn : [ 35.725190] ? kernfs_put (arch/x86/include/asm/atomic.h:123 (discriminator 1) include/linux/atomic/atomic-instrumented.h:576 (discriminator 1) fs/kernfs/dir.c:536 (discriminator 1))
kern :warn : [ 35.729449] pci_device_probe (drivers/pci/pci-driver.c:461)
kern :warn : [ 35.734150] really_probe (drivers/base/dd.c:560 drivers/base/dd.c:639)
kern :warn : [ 35.738596] __driver_probe_device (drivers/base/dd.c:719 drivers/base/dd.c:776)
kern :warn : [ 35.743827] driver_probe_device (drivers/base/dd.c:808)
kern :warn : [ 35.748787] __driver_attach (drivers/base/dd.c:1191)
kern :warn : [ 35.753494] ? __device_attach_driver (drivers/base/dd.c:1135)
kern :warn : [ 35.758968] bus_for_each_dev (drivers/base/bus.c:301)
kern :warn : [ 35.763741] ? subsys_dev_iter_exit (drivers/base/bus.c:290)
kern :warn : [ 35.768860] ? __kmem_cache_alloc_node (mm/slub.c:3400 mm/slub.c:3437)
kern :warn : [ 35.774388] ? klist_add_tail (include/linux/list.h:69 include/linux/list.h:102 lib/klist.c:104 lib/klist.c:137)
kern :warn : [ 35.779157] bus_add_driver (drivers/base/bus.c:618)
kern :warn : [ 35.783759] driver_register (drivers/base/driver.c:246)
kern :warn : [ 35.788442] i915_init (drivers/gpu/drm/i915/i915_driver.c:1078) i915
kern :warn : [ 35.793347] ? 0xffffffffa14e3000
kern :warn : [ 35.797404] do_one_initcall (init/main.c:1303)
kern :warn : [ 35.802026] ? trace_event_raw_event_initcall_level (init/main.c:1294)
kern :warn : [ 35.808719] ? __asan_register_globals (mm/kasan/generic.c:214 (discriminator 3) mm/kasan/generic.c:226 (discriminator 3))
kern :warn : [ 35.814120] ? kasan_unpoison (mm/kasan/shadow.c:108 mm/kasan/shadow.c:142)
kern :warn : [ 35.818735] do_init_module (kernel/module/main.c:2455)
kern :warn : [ 35.823348] load_module (kernel/module/main.c:2849)
kern :warn : [ 35.827859] ? post_relocation (kernel/module/main.c:2673)
kern :warn : [ 35.832716] ? __x64_sys_fspick (fs/kernel_read_file.c:38)
kern :warn : [ 35.837666] ? do_mmap (mm/mmap.c:1412)
kern :warn : [ 35.841947] ? __do_sys_finit_module (kernel/module/main.c:2949)
kern :warn : [ 35.847338] __do_sys_finit_module (kernel/module/main.c:2949)
kern :warn : [ 35.852552] ? __ia32_sys_init_module (kernel/module/main.c:2917)
kern :warn : [ 35.857850] ? fput (arch/x86/include/asm/atomic64_64.h:118 include/linux/atomic/atomic-long.h:467 include/linux/atomic/atomic-instrumented.h:1814 fs/file_table.c:371)
kern :warn : [ 35.861675] do_syscall_64 (arch/x86/entry/common.c:50 arch/x86/entry/common.c:80)
kern :warn : [ 35.866017] entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
kern :warn : [ 35.871828] RIP: 0033:0x7f9cb95a33a9
kern :warn : [ 35.876164] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d b7 8a 0d 00 f7 d8 64 89 01 48
All code
========
0: 00 c3 add %al,%bl
2: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
9: 00 00 00
c: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a:* 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 8b 0d b7 8a 0d 00 mov 0xd8ab7(%rip),%rcx # 0xd8af1
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W

Code starting with the faulting instruction
===========================================
0: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
6: 73 01 jae 0x9
8: c3 retq
9: 48 8b 0d b7 8a 0d 00 mov 0xd8ab7(%rip),%rcx # 0xd8ac7
10: f7 d8 neg %eax
12: 64 89 01 mov %eax,%fs:(%rcx)
15: 48 rex.W



If you fix the issue, kindly add following tag
| Reported-by: kernel test robot <[email protected]>
| Link: https://lore.kernel.org/oe-lkp/[email protected]


To reproduce:

git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
sudo bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
sudo bin/lkp run generated-yaml-file

# if come across any failure that blocks the test,
# please remove ~/.lkp and /lkp dir to run from a clean state.


--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (12.17 kB)
config-6.1.0-rc8-00063-g22f82cbe7636 (173.85 kB)
job-script (6.11 kB)
kmsg.xz (41.68 kB)
xfstests (1.16 kB)
job.yaml (4.95 kB)
reproduce (0.98 kB)
Download all attachments

2022-12-11 22:22:49

by Bartosz Golaszewski

[permalink] [raw]
Subject: Re: [PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel

On Sun, Dec 11, 2022 at 5:32 PM kernel test robot <[email protected]> wrote:
>
> Greeting,
>
> FYI, we noticed WARNING:at_drivers/base/core.c:#device_release due to commit (built with gcc-11):
>
> commit: 22f82cbe7636f59cc21f6837c500d704e3b412a5 ("[PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel")
> url: https://github.com/intel-lab-lkp/linux/commits/Bartosz-Golaszewski/i2c-fortify-the-subsystem-against-user-space-induced-deadlocks/20221209-022450
> base: https://git.kernel.org/cgit/linux/kernel/git/wsa/linux.git i2c/for-next
> patch link: https://lore.kernel.org/all/[email protected]/
> patch subject: [PATCH 2/2] i2c: dev: don't allow user-space to deadlock the kernel
>
> in testcase: xfstests
> version: xfstests-x86_64-5a5e419-1_20221128
> with following parameters:
>
> disk: 4HDD
> fs: xfs
> test: xfs-group-07
>
> test-description: xfstests is a regression test suite for xfs and other files ystems.
> test-url: git://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git
>
> on test machine: 4 threads 1 sockets Intel(R) Core(TM) i3-3220 CPU @ 3.30GHz (Ivy Bridge) with 8G memory
>
> caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
>
>
> kern :warn : [ 35.378637] ------------[ cut here ]------------
> kern :err : [ 35.388198] Device 'i2c-7' does not have a release() function, it is broken and must be fixed. See Documentation/core-api/kobject.rst.
> kern :warn : [ 35.401052] WARNING: CPU: 1 PID: 195 at drivers/base/core.c:2332 device_release (drivers/base/core.c:2332)
> kern :warn : [ 35.410178] Modules linked in: x86_pkg_temp_thermal intel_powerclamp i915(+) sd_mod t10_pi crc64_rocksoft_generic crc64_rocksoft ipmi_devintf crc64 coretemp ipmi_msghandler sg kvm_intel drm_buddy intel_gtt kvm irqbypass crct10dif_pclmul drm_display_helper crc32_pclmul ttm crc32c_intel ghash_clmulni_intel drm_kms_helper sha512_ssse3 syscopyarea ahci mei_me rapl intel_cstate libahci sysfillrect wmi_bmof sysimgblt i2c_i801 mei intel_uncore libata i2c_smbus lpc_ich fb_sys_fops video wmi drm fuse ip_tables
> kern :warn : [ 35.455372] CPU: 1 PID: 195 Comm: systemd-udevd Not tainted 6.1.0-rc8-00063-g22f82cbe7636 #1
> kern :warn : [ 35.464544] Hardware name: Hewlett-Packard HP Pro 3340 MT/17A1, BIOS 8.07 01/24/2013
> kern :warn : [ 35.473036] RIP: 0010:device_release (drivers/base/core.c:2332)
> kern :warn : [ 35.478405] Code: 48 8d 7d 50 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 89 00 00 00 48 8b 75 50 48 85 f6 74 13 48 c7 c7 60 f0 ce 83 e8 6c 1c f2 00 <0f> 0b e9 0f ff ff ff 48 b8 00 00 00 00 00 fc ff df 48 89 ea 48 c1
> All code

Ah yes, the .release() callback must stay and while the adapter
resources are handled by the bus driver, we still need to free the idr
at device release time.

I'll fix that in v2 but I also want to wait for some comments.

Bart