kernel game #3

Assume we have an extremely buggy driver. Multiple threads can call into meh_ioctl shown below at the same time with the same device and there is no locking provided. The routine is supposed to either store a pointer to a referenced struct file object in m->fp or just clear the entry (and of course get rid of the reference).

What can go wrong here? Consider both a singlethraded and multithreaded execution.

int meh_ioctl(dev_t dev, ioctl_t ioct, int data)
{
        meh_t m *m = to_meh(dev);
        struct file *fp;

        switch (ioct) {
        case MEH_ATTACH:
                /* data is the fd we are going to borrow the file from */

                /* check if we already have a reference to a file */
                if (m->fp != NULL)
                        frele(m->fp);
                /* fget return the file with a reference or NULL on error */
                fp = fget(data);
                if (fp == NULL)
                        return EBADF:
                m->fp = fp;
                break;
        case MEH_DETACH:
                if (m->fp == NULL)
                        return EINVAL;
                frele(m->fp);
                m->fp = NULL;
                break;
        }

        return 0;
}

This is extremely broken and sometimes in not so obvious ways.

We can start with a possible bug with only one thread executing the function. Assume m->fp != NULL is true. frele is executed. Then fget(data) is executed and this possibly returns an error. But m->fp was not cleared, despite the reference being dropped.

Now let's see what can happen with multiple threads. Races are too numerous to describe in great detail, so we will only consider few of them.
1. concurrent MEH_DETACH

MEH_DETACH                              MEH_DETACH
CPU0                                    CPU1
...                                     ...
if (m->fp == NULL) /* false */          if (m->fp == NULL) /* false */
frele(m->fp);                           frele(m->fp);

Or, to state differently, both CPUs can conclude the pointer is not NULL and then proceed to call frele, resulting in the object being overput.

2. concurrent MEH_ATTACH

Similarly to the previous case, the code can drop more references than it had.

But also:


MEH_ATTACH                      MEH_ATTACH
CPU0                            CPU1
...                             ...
fp = fget(data);                fp = fget(data);
...                             ...
m->fp = fp;                            
                                m->fp = fp;
                                       
Here we got the same object and it is effectively referenced twice,
but MEH_DETACH will be only able to drop one reference, thus making the driver leak another one.

3. concurrent MEH_ATTACH and MEH_DETACH

MEH_ATTACH                      MEH_DETACH
CPU0                            CPU1
...                             ...
if (m->fp)                      if (m->fp == NULL) /* false */                                 ...
  frele(m->fp);                 frele(m->fp);
       
So similarly to previous cases the code can overput the object.

MEH_ATTACH                      MEH_DETACH
CPU0                            CPU1
...                             ...
fp = fget(data);                frele(m->fp);
...
m->fp = fp;
                                m->fp = NULL;
                                       
Here we got the reference to the object and assigned it to m->fp, but the pointer was cleared.

4. concurrent MEH_ATTACH and a thread changing the fd

Here another thread will try to change the file which can be obtained with passed fd.

MEH_ATTACH                      MEH_ATTACH                     fd
CPU0                            CPU1                           CPU2
...                             ...
fp = fget(data); /* file1 */       
                                                               the file
                                                               is
                                                               replaced
...                             fp = fget(data); /* file2 */      
m->fp = fp;                     ...
                                m->fp = fp;
                                       
Here we leak the reference to file1 since m->fp now points to file2 and nothing cleared the reference in the meantime.

5. what about actual m->fp users?

One can note the driver stores the pointer for a reason. Given the lack of synchronisation this basically means use-after-free and NULL pointer dereferences, depending on the timing.

The fix? Locking.