Linux Device Driver Access Control

Question

I am implementing a device driver for some novel hardware, and want to only allow one process to have access to the device at a time. Concurrent read/write operations would confuse the hardware to the point where a hard reset would more than likely need to occur. I still have the following questions:

1. In the example code from Linux Device Drivers , the open() call uses a lock, but close() does not. Isn't there still a race condition here, or is the decrement of scull_s_count guaranteed to be atomic? Basically, in this example, I'm wondering what would happen if one process is trying to open the device right when another process is wrapping up and closing it.

2. I'm assuming that I do not need to check the status of my open flag (I'm doing something similar to the example's scull_s_count) in my read() and write() calls, since the only way to get into these calls is if the userspace application already has received the fd via a successful call to open(). Is that assumption correct?

Thanks to tadman's comments, I did the most cursory search of the kernel's atomic_t mechanisms. Here's some pseudo-code of what I have now:

int open(struct inode *inode, struct file *filp) {
  spin_lock(&lock);
  if (atomic_read(&open_flag)) {
    spin_unlock(&lock);
    return -EBUSY;
  }
  atomic_set(&open_flag, 1);
  /* do other open() related stuff */
  spin_unlock(&lock);
  return 0;
}

int close(struct inode *inode, struct file *filp) {
  int rc;
  /* do close() stuff */
  atomic_set(&open_flag, 0);
  return rc;
}

The open_flag is an atomic_t that is part of a larger struct that is allocated with kzalloc(). As a result, it gets initialized to zero.

As a result, the code here shows that the lock's purpose is to prevent multiple processes/threads from opening the device at the same time, while the fact that the open_flag is an atomic_t prevents the race condition I was concerned about in question 1 above. Does this implementation suffice? Also, I am still looking for an answer to question 2.

The example code uses a spinlock, but would a mutex be more appropriate? The section of code is relatively small and there is little to no contention, so going to sleep and waking up might have less performance than just spinning. The lock/mutex is always accessed from the user context, so you should be safe to sleep.

Answer 1

The example you point to is indeed flawed. The decrement is absolutely not guaranteed to be atomic and almost certainly will not be.

But realistically, I don't think there is a compiler/CPU combination that would ever produce code that could fail. The worst that could happen is one CPU core could finish the close, then another core could call open and get back busy because it has a stale cached value of the flag.

Linux provides atomic_* functions for this and also *_bit atomic bit flag operations. See core_api/atomic_ops.rst in the kernel documentation.

A example of a correct and simple pattern to do his would look something like:

unsigned long mydriver_flags;
#define IN_USE_BIT 0

static int mydriver_open(struct inode *inode, struct file *filp)
{
        if (test_and_set_bit(IN_USE_BIT, &mydriver_flags))
                return -EBUSY;
        /* continue with open */
        return 0;
}

static int mydriver_close(struct inode *inode, struct file *filp)
{
        /* do close stuff first */
        smp_mb__before_atomic();
        clear_bit(IN_USE_BIT, &mydriver_flags);
        return 0;
}

A real driver should have a device state struct, for each device, with mydriver_flags in it. Rather than using a single global for the whole driver as shown in the example.

That said, what you are trying to do is probably not a good idea. Even if only one process can open the device at a time, a process' open file descriptors are shared amongst all threads in the process. Multiple threads can make simultaneous read() and write() calls to the same file descriptor.

If a process has a file descriptor open and calls fork(), that descriptor will be inherited into the new process. This is a way multiple processes can have the device open at once despite the above "single open" restriction.

So you still have to be thread safe in your driver's file operations since a user can still have multiple threads/processes open the device at once and make simultaneous calls. And if you've made it safe, why prevent the user from doing it? Maybe they know what they're doing and will insure their multiple openers of the driver will "take turns" and not make calls that conflict?

Also consider the possibility using the O_EXCL flag in the open call to make single open optional.