Is there any possibility of a race condition in this code?

Question

I'm programming a buffer that should not be accessed concurrently. Is there any possibility that it happens? The code just creates a buffer that will be accessed by many programs and that acts as a FIFO queue. I'm using kernel semaphores that I think are well initialized. ¿How could I try this code?

#define MAX_BUFFER_SIZE 1024

#include "cbuffer.h"
#include <linux/string.h>
#include <asm-generic/uaccess.h>
#include <asm-generic/errno.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/fs.h>
#include <linux/semaphore.h>

MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("Driver Module for DSO");
MODULE_AUTHOR("Kaostias");

/*  
 *  Prototypes - this would normally go in a .h file
 */
int init_module(void);
void cleanup_module(void);
static int device_open(struct inode *, struct file *);
static int device_release(struct inode *, struct file *);
static ssize_t device_read(struct file *, char *, size_t, loff_t *);
static ssize_t device_write(struct file *, const char *, size_t, loff_t *);

#define SUCCESS 0
#define DEVICE_NAME "fifodev"   /* Dev name as it appears in /proc/devices   */
#define BUF_LEN 512         /* Max length of the message from the device */

/* 
 * Global variables are declared as static, so are global within the file. 
 */

static int Major;       /* Major number assigned to our device driver */
static int Device_Open = 0; /* Is device open?  
                 * Used to prevent multiple access to device */

cbuffer_t * buf;

struct semaphore prod_queue,cons_queue;
struct semaphore mtx;
int nr_prod_init, nr_cons_init;

int nr_prod_waiting,nr_cons_waiting;

int nr_producers,nr_consumers;
int nr_active_prod, nr_active_con;

static struct file_operations fops = {
    .read = device_read,
    .write = device_write,
    .open = device_open,
    .release = device_release
};

/*
 * This function is called when the module is loaded
 */
int init_module(void)
{
        Major = register_chrdev(0, DEVICE_NAME, &fops);

    if (Major < 0) {
      printk(KERN_ALERT "Registering char device failed with %d\n", Major);
      return -Major;
    }
    /*
     * Creation of buffer;
     */
    if( (buf = create_cbuffer_t(MAX_BUFFER_SIZE)) == NULL){
        printk(KERN_ALERT "Error when creating the FIFO device.");
        return -EINVAL;
    }
    sema_init(&prod_queue,0);
    sema_init(&cons_queue,0);
    sema_init(&mtx,1);
    nr_prod_waiting=0;
    nr_cons_waiting=0;

    nr_active_prod = 0;
    nr_active_con =0;

    nr_producers=0;
    nr_consumers=0;

    printk(KERN_INFO "Buffer created without error.\n");
    printk(KERN_INFO "I was assigned major number %d. To talk to\n", Major);
    printk(KERN_INFO "the driver, create a dev file with\n");
    printk(KERN_INFO "'mknod /dev/%s c %d 0'.\n", DEVICE_NAME, Major);
    printk(KERN_INFO "Try various minor numbers. Try to cat and echo to\n");
    printk(KERN_INFO "the device file.\n");
    printk(KERN_INFO "Remove the device file and module when done.\n");

    return SUCCESS;
}

/*
 * This function is called when the module is unloaded
 */
void cleanup_module(void)
{
    /* 
     * Unregister the device 
     */
    /*int ret = */unregister_chrdev(Major, DEVICE_NAME);
/*  if (ret < 0)
        printk(KERN_ALERT "Error in unregister_chrdev\n");//, ret);*/
    /*
     * Destroys the FIFO buffer
     */
    destroy_cbuffer_t (buf);

}

/*
 * Methods
 */

/* 
 * Called when a process tries to open the device file, like
 * "cat /dev/mycharfile"
 */
static int device_open(struct inode *inode, struct file *file)
{
    static int counter = 0;
    printk(KERN_ALERT "Entrando a Device_Open");

    if (down_interruptible(&mtx)) /*BLOQUEO*/
        return -EINTR;


    if (file->f_mode & FMODE_READ){
        nr_consumers++;
        while(nr_producers == 0){
            up(&mtx);
            /*Espera*/
            if (down_interruptible(&cons_queue)){
                down(&mtx);
                nr_consumers--;
                up(&mtx);       
                return -EINTR;
            }
            if (down_interruptible(&mtx))
                return -EINTR;
        }
        nr_active_prod++;
//      up(&mtx);
        if(nr_active_con == 0){

            up(&prod_queue);
        }

    } else {
        nr_producers++;
        while(nr_consumers == 0){
            up(&mtx);
            /*Espera*/
            if(down_interruptible(&prod_queue)){
                down(&mtx);
                nr_producers--;
                up(&mtx);
                return -EINTR;
            }
            if (down_interruptible(&mtx))
                return -EINTR;
        }
        nr_active_con++;
//      up(&mtx);

        if(nr_active_prod == 0){
            up(&cons_queue);
        }
    }

    Device_Open++;
    printk(KERN_ALERT "The device %s has been open %d times.\n",DEVICE_NAME ,++counter);
    try_module_get(THIS_MODULE);
    printk(KERN_ALERT "activos: Productores-%d; Consumidores-%d",nr_active_prod,nr_active_con);
    up(&mtx); /*Fin bloqueo*/

    printk(KERN_ALERT "Saliendo de device_Open");
    return SUCCESS;
}

/* 
 * Called when a process closes the device file.
 */
static int device_release(struct inode *inode, struct file *file)
{
    printk(KERN_ALERT "Entrando a device_release");

    if (down_interruptible(&mtx)){
        return -EINTR;
    }   /*BLOQUEO*/
    if (file->f_mode & FMODE_READ){
        nr_active_con--;
    }else{
        nr_active_prod--;
    }
    Device_Open--;      /* We're now ready for our next caller */
    printk(KERN_ALERT "hay %d dispositivos abiertos", Device_Open);
    module_put(THIS_MODULE);

    up(&mtx);  /*Fin bloqueo*/

    printk(KERN_ALERT "Saliendo de device_release");
    return SUCCESS;
}

/* 
 * Called when a process, which already opened the dev file, attempts to
 * read from it.
 */
static ssize_t device_read(struct file *filp,   /* see include/linux/fs.h   */
               char *buffer,    /* buffer to fill with data */
               size_t length,   /* length of the buffer     */
               loff_t * offset)
{
    char aux[BUF_LEN];
    printk(KERN_ALERT "Entrando a device_read");

    /*if (length > BUF_LEN) 
        return -EINVAL;*/
    /*BLOQUEO*/
    if (down_interruptible(&mtx)){
        return -EINTR;
    }

    if (nr_active_prod==0 && size_cbuffer_t(buf)<length){//is_empty_cbuffer_t(buf)){
        up(&mtx);
        return 0;
    }
    while(size_cbuffer_t(buf)<length){
        nr_cons_waiting++;
        printk(KERN_ALERT "Bloqueo sideral, cons: %d, prod: %d",nr_cons_waiting,nr_prod_waiting);
        printk (KERN_ALERT "Tam_buffer-%d, tamCadena-%d",size_cbuffer_t(buf),length);
        up(&mtx);

        /* Bloqueo en cola de espera */     
        if (down_interruptible(&cons_queue)){
            down(&mtx);
            nr_cons_waiting--;
            up(&mtx);       
            return -EINTR;
        }

    /* Readquisición del 'mutex' antes de entrar a la SC */     
        if (down_interruptible(&mtx)){
            return -EINTR;
        }
        if (nr_active_prod==0 && size_cbuffer_t(buf)<length){
            up(&mtx);
            return 0;
        }   

    }

    remove_items_cbuffer_t (buf,aux, length);//length);

    if (nr_prod_waiting>0){
        up(&prod_queue);    
        nr_prod_waiting--;
    }

    /* Salir de la sección crítica */   
    up(&mtx);
    /*Fin bloqueo*/
    if(copy_to_user(buffer, aux, length)){
        printk(KERN_ALERT "error en copy_to_user");
        return -EINVAL;
    }

    printk(KERN_ALERT "Saliendo de device_read");
    return length;
}

/*  
 * Called when a process writes to dev file: echo "hi" > /dev/hello 
 */
static ssize_t
device_write(struct file *filp, const char *buff, size_t len, loff_t * off)
{
    char aux[BUF_LEN];
    printk(KERN_ALERT "Entrando a device_write");

    if (len>BUF_LEN){
        printk(KERN_ALERT "Error, la longitud del buffer es excesiva, es de %d", len);
        return -ENOSPC;
    }

    if(copy_from_user(aux,buff,len)){
        printk(KERN_ALERT "Problemas en copy from user");
        return -EFAULT;
    }
    /*BLOQUEO*/
    if (down_interruptible(&mtx)){
        printk(KERN_ALERT "Problemas en bloqueo");  
        return -EINTR;
    }

    if (nr_consumers==0){
        up(&mtx);
        return -EFAULT;
    }

    while(nr_gaps_cbuffer_t(buf)<len){

        /*Se aumenta el número de productores esperando 
          y se levanta el bloqueo*/
        nr_prod_waiting++;
        up(&mtx);
        /* Se ponea dormir el proceso hasta que alguien lo despierte
        */
        if (down_interruptible(&prod_queue)){
            printk(KERN_ALERT "Problemas en bloqueo2");
            down(&mtx);
            nr_prod_waiting--;
            up(&mtx);       
            return -EINTR;
        }
        /* Una vez se ha despertado, se bloquea de nuevo
           (bloqueo general)*/
        if (down_interruptible(&mtx)){
            return -EINTR;
        }

        if (nr_consumers==0){
            up(&mtx);
            return -EFAULT;
        }   
    }

    insert_items_cbuffer_t(buf, aux, len);

    /* Despertar a los productores bloqueados (si hay alguno) */
      if (nr_cons_waiting>0){
        up(&cons_queue);    
        nr_cons_waiting--;
      }

      up(&mtx); /*Fin bloqueo*/

    aux[len] = '\0';
    printk(KERN_ALERT "Saliendo de device_write, se han escrito %d bytes",len);
    return len;
}

Answer 1

The best way to test for race conditions is to add random sleeps in the code. In that case when you run the code multiple times you increased the chance of hitting an error due to a race condition. Using random sleeps to ensure the behavior is different every run. Sometimes reading code to find these type of errors is just too much work, and testing is easier.