Seems like we are not lock-free

1 files changed, 29 insertions, 19 deletions

diff --git a/mod_add.c b/mod_add.c
index c259605..096baaf 100644
--- a/mod_add.c
+++ b/mod_add.c
@@ -5,6 +5,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/cdev.h>
+#include <linux/semaphore.h>
 
 MODULE_LICENSE("GPL");
 
@@ -22,7 +23,8 @@ static int device_open_count = 0;
 struct sad_buf {
     char *buf;
     size_t size;
-};
+    struct semaphore lock;
+} *buf;
 
 static struct file_operations sad_fops = {
     .owner = THIS_MODULE,
@@ -34,64 +36,63 @@ static struct file_operations sad_fops = {
 
 
 static ssize_t sad_read(struct file *flip, char __user *user_buf, size_t len, loff_t *offset) {
-    struct sad_buf *buf = flip->private_data;
     size_t bytes_left, read_size, uncopied;
 
-    printk(KERN_INFO "Getting offset %llu\t buf size: %zu\n", *offset, buf->size);
+    if (down_interruptible(&buf->lock))
+        return -ERESTARTSYS;
 
-    if (*offset >= buf->size)
+    if (*offset >= buf->size) {
+        up(&buf->lock);
         return 0;
+    }
 
     bytes_left = buf->size - *offset;
     read_size = min(bytes_left, len);
     uncopied = copy_to_user(user_buf, buf->buf + *offset, read_size);
 
     *offset += read_size - uncopied;
-    printk(KERN_INFO "off: %llu\t uncopied: %zu\t ret: %zu\n", *offset, uncopied, read_size - uncopied);
+
+    up(&buf->lock);
 
     return read_size - uncopied;
 }
 
 static ssize_t sad_write(struct file *flip, const char __user *user_buf, size_t len, loff_t *offset) {
-    struct sad_buf *buf = flip->private_data;
     size_t uncopied;
 
+    if (down_interruptible(&buf->lock))
+        return -ERESTARTSYS;
+
     if (buf->buf)
         kfree(buf->buf);
 
     buf->buf = kmalloc(len, GFP_KERNEL);
-    if (!buf->buf)
+    if (!buf->buf) {
+        up(&buf->lock);
         return -ENOMEM;
+    }
 
     buf->size = len;
 
     *offset = 0;
 
     uncopied = copy_from_user(buf->buf, user_buf, len);
-    printk(KERN_INFO "uncopied: %zu\t ret: %zu\n", uncopied, len - uncopied);
+
+    up(&buf->lock);
 
     return len - uncopied;
 }
 
 static int sad_open(struct inode *inode, struct file *file) {
-    struct sad_buf *buf = kzalloc(sizeof(*buf), GFP_KERNEL);
-    if (!buf)
-        return -ENOMEM;
-
-    file->private_data = buf;
-
     device_open_count++;
     try_module_get(THIS_MODULE);
     return 0;
 }
 
 static int sad_release(struct inode *inode, struct file *file) {
-    struct sad_buf *buf = file->private_data;
-    if (buf->buf)
-        kfree(buf->buf);
-    kfree(buf);
     device_open_count--;
-    module_put(THIS_MODULE);
+    if (device_open_count == 0)
+        module_put(THIS_MODULE);
     return 0;
 }
 
@@ -101,6 +102,12 @@ static int __init sad_init(void) {
     const unsigned int minor = 0;
     const unsigned int count = 1;
 
+    buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+    if (!buf)
+        return -ENOMEM;
+
+    sema_init(&buf->lock, 1);
+
     rc = alloc_chrdev_region(&dev, minor, count, DEV_NAME);
     if (rc < 0) {
         printk(KERN_ALERT "Could not register device: %d\n", rc);
@@ -123,6 +130,9 @@ static int __init sad_init(void) {
 }
 static void __exit sad_exit(void) {
     cdev_del(cdev);
+    if (buf->buf)
+        kfree(buf->buf);
+    kfree(buf);
     unregister_chrdev_region(dev, 1);
     printk(KERN_INFO "okay, bye\n");
 }