[Question] Can forked file descriptions lead to race condition in chardev2.c?

[QiuhaoLi]

In chardev2.c, Compare-And-Swap operation is used in device_open() to avoid concurrent access to the shared resources:

/* This is called whenever a process attempts to open the device file */ 
static int device_open(struct inode *inode, struct file *file) 
{ 
    pr_info("device_open(%p)\n", file); 
 
    /* We don't want to talk to two processes at the same time. */ 
    if (atomic_cmpxchg(&already_open, CDEV_NOT_USED, CDEV_EXCLUSIVE_OPEN)) 
        return -EBUSY; 
 
    try_module_get(THIS_MODULE); 
    return SUCCESS; 
} 

But what if a process opens the device, then triggers a fork(2). Can these two processes have concurrent access to the device? (we called open(2) only once)

Thanks.

[linD026]

Hi @QiuhaoLi

According to fork() manual page, since the child refers to the same open file description, it will have concurrent access while calling open() before fork().

 *  The child inherits copies of the parent's set of open file descriptors.  Each file  descrip‐
    tor in the child refers to the same open file description (see open(2)) as the corresponding
    file descriptor in the parent.  This means that the two file  descriptors  share  open  file
    status flags, file offset, and signal-driven I/O attributes (see the description of F_SETOWN
    and F_SETSIG in fcntl(2)).

Since the read(), write(), lseek() syscall become atomic, it is no problem here. You can see the discussion here:

• Add the description about some file ops and fix several problems #108

However, under this situation ( calling open() before fork() ), there will have a race condition in device_ioctl() although we did the check in device_open().

---

The reason why we will check the concurrent access to the device at device_open() is
the original code already exists for this kind of checking. And, we prefer to preserve the original code.

/* 
 * This is called whenever a process attempts to open the device file 
 */
static int device_open(struct inode *inode, struct file *file)
{
#ifdef DEBUG
	printk(KERN_INFO "device_open(%p)\n", file);
#endif

	/* 
	 * We don't want to talk to two processes at the same time 
	 */
	if (Device_Open)
		return -EBUSY;

	Device_Open++;
	/*
	 * Initialize the message 
	 */
	Message_Ptr = Message;
	try_module_get(THIS_MODULE);
	return SUCCESS;
}

---

In conclusion, there is a problem with chardev2.c.
It seems like it doesn't need the check in device_open() anymore. Because:

• The read(), write(), lseek() work well for concurrent access, since they are atomic.
• Because of fork(), even though we exclusively open the device file, it still has a race condition in device_ioctl().

I will fix it as soon as possible.

Thanks.

[QiuhaoLi]

@linD026 @jserv

Thank you so much for such a quick and detailed response :)