Skip to content

Commit beb1ff1

Browse files
committed
Fix potential concurrent problems in chardev2.c
After forking, Each file descriptor in the child refers to the same open file description as the parent. So when calling open() before fork(), the child can access the device file without checking by exclusive access in device_open(). It may cause race conditions in device_ioctl(). Because of that, it is unnecessary to check the multiple access in device_open(). It just needs check in device_ioctl(), since read(), write(), seek() system call are atomic [1][2]. Related discussion: - #148 [1] https://lore.kernel.org/lkml/[email protected]/ [2] https://www.kernel.org/doc/html/latest/filesystems/files.html Close #148
1 parent aa4b5d7 commit beb1ff1

File tree

1 file changed

+10
-9
lines changed

1 file changed

+10
-9
lines changed

examples/chardev2.c

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,6 @@ static int device_open(struct inode *inode, struct file *file)
3737
{
3838
pr_info("device_open(%p)\n", file);
3939

40-
/* We don't want to talk to two processes at the same time. */
41-
if (atomic_cmpxchg(&already_open, CDEV_NOT_USED, CDEV_EXCLUSIVE_OPEN))
42-
return -EBUSY;
43-
4440
try_module_get(THIS_MODULE);
4541
return SUCCESS;
4642
}
@@ -49,9 +45,6 @@ static int device_release(struct inode *inode, struct file *file)
4945
{
5046
pr_info("device_release(%p,%p)\n", inode, file);
5147

52-
/* We're now ready for our next caller */
53-
atomic_set(&already_open, CDEV_NOT_USED);
54-
5548
module_put(THIS_MODULE);
5649
return SUCCESS;
5750
}
@@ -129,6 +122,11 @@ device_ioctl(struct file *file, /* ditto */
129122
unsigned long ioctl_param)
130123
{
131124
int i;
125+
long ret = SUCCESS;
126+
127+
/* We don't want to talk to two processes at the same time. */
128+
if (atomic_cmpxchg(&already_open, CDEV_NOT_USED, CDEV_EXCLUSIVE_OPEN))
129+
return -EBUSY;
132130

133131
/* Switch according to the ioctl called */
134132
switch (ioctl_num) {
@@ -166,11 +164,14 @@ device_ioctl(struct file *file, /* ditto */
166164
/* This ioctl is both input (ioctl_param) and output (the return
167165
* value of this function).
168166
*/
169-
return (long)message[ioctl_param];
167+
ret = (long)message[ioctl_param];
170168
break;
171169
}
172170

173-
return SUCCESS;
171+
/* We're now ready for our next caller */
172+
atomic_set(&already_open, CDEV_NOT_USED);
173+
174+
return ret;
174175
}
175176

176177
/* Module Declarations */

0 commit comments

Comments
 (0)