Patch for mmap Bug in dazukofs 3.1.0-rc1

[Lino Sanfilippo]

Hi all,

although the new mmap() approach (deny writing, allow reading) used by
dazukofs 3.1.0-rc1 works very fine so far (the BUG reported on dazukofs
help mailing list:
http://lists.gnu.org/archive/html/dazuko-help/2009-04/msg00008.html
seems to be fixed), there is still a litte flaw:

If the underlaying filesystem does not support mmap() for a regular file
or a directory,
we must not call generic_file_readonly_mmap() for this file type:
Calling this function always results in calling the lower address_space
objects readpage()
(see read_cache_page() in mmap.c).  But this might not be initialized if
no mmap() is supported,
so we run into an Oops in this case.

This bug can simply be triggered by mounting dazukofs over an ext3 -
which does not provide
mmaping for directories - and reading an arbitrary directory via mmap.

Please see also the description for patch 2 for dazukofs release 3.0.0.

This patch ensures that generic_file_readonly_mmap() is only called if
the lower file actually
supports mmap.
If not the error code -ENODEV, which is used by the vfs to indicate that
mmap is not supported,
is returned.


Regards,
Lino Sanfilippo




Geschäftsführender Gesellschafter: Tjark Auerbach
Sitz der Gesellschaft: Tettnang
Handelsregister: Amtsgericht Ulm, HRB 630992
ALLGEMEINE GESCHÄFTSBEDINGUNGEN
Es gelten unsere Allgemeinen Geschäftsbedingungen
(AGB). Sie finden sie in der jeweils gültigen Fassung
im Internet unter http://www.avira.de/agb
***************************************************
diff -rup dazukofs-3.1.0-rc1/file.c dazukofs-3.1.0-rc1-p1/file.c
--- dazukofs-3.1.0-rc1/file.c 2009-06-28 22:17:20.000000000 +0200
+++ dazukofs-3.1.0-rc1-p1/file.c 2009-07-01 17:54:39.000000000 +0200
@@ -27,6 +27,7 @@
 #include <linux/file.h>
 #include <linux/fs_stack.h>
 #include <linux/cred.h>
+#include <linux/sched.h>
 
 #include "dazukofs_fs.h"
 #include "event.h"
@@ -256,6 +257,19 @@ static int dazukofs_fasync(int fd, struc
  return lower_file->f_op->fasync(fd, lower_file, flag);
 }
 
+
+static int dazukofs_mmap(struct file *file, struct vm_area_struct *vm)
+{
+ struct file *lower_file = get_lower_file(file);
+
+ /* if lower fs does not support mmap, we dont call generic_mmap(), since
+ * this would result in calling lower readpage(), which might not be defined
+ * by lower fs, since mmap is not supported */
+ if (!lower_file->f_op || !lower_file->f_op->mmap)
+ return -ENODEV;
+ return generic_file_mmap(file, vm);
+}
+
 /**
  * Unused operations:
  *   - owner
@@ -284,7 +298,7 @@ const struct file_operations dazukofs_ma
  .aio_write = generic_file_aio_write,
  .readdir = dazukofs_readdir,
  .ioctl = dazukofs_ioctl,
- .mmap = generic_file_readonly_mmap,
+ .mmap = dazukofs_mmap,
  .open = dazukofs_open,
  .flush = dazukofs_flush,
  .release = dazukofs_release,
@@ -320,7 +334,7 @@ const struct file_operations dazukofs_di
  .read = dazukofs_read,
  .readdir = dazukofs_readdir,
  .ioctl = dazukofs_ioctl,
- .mmap = generic_file_readonly_mmap,
+ .mmap = dazukofs_mmap,
  .open = dazukofs_open,
  .flush = dazukofs_flush,
  .release = dazukofs_release,
diff -rup dazukofs-3.1.0-rc1/ign_dev.c dazukofs-3.1.0-rc1-p1/ign_dev.c
--- dazukofs-3.1.0-rc1/ign_dev.c 2009-06-28 22:16:23.000000000 +0200
+++ dazukofs-3.1.0-rc1-p1/ign_dev.c 2009-07-01 17:56:10.000000000 +0200
@@ -26,6 +26,7 @@
 
 #include "dazukofs_fs.h"
 #include "dev.h"
+#include <linux/sched.h>
 
 struct dazukofs_proc {
  struct list_head list;

_______________________________________________
Dazuko-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/dazuko-devel

[John Ogness-5]

On 2009-07-01, Lino Sanfilippo <[hidden email]> wrote:

> This patch ensures that generic_file_readonly_mmap() is only called
> if the lower file actually supports mmap.
>
> If not the error code -ENODEV, which is used by the vfs to indicate
> that mmap is not supported, is returned.
>
> +static int dazukofs_mmap(struct file *file, struct vm_area_struct *vm)
> +{
> + struct file *lower_file = get_lower_file(file);
> +
> + /* if lower fs does not support mmap, we dont call generic_mmap(), since
> + * this would result in calling lower readpage(), which might not be defined
> + * by lower fs, since mmap is not supported */
> + if (!lower_file->f_op || !lower_file->f_op->mmap)
> + return -ENODEV;
> + return generic_file_mmap(file, vm);
> +}

Shouldn't it be:

    if (!lower_file->f_op || !lower_file->f_op->mmap)
            return -ENODEV;
    return generic_file_readonly_mmap(file, vm);

instead?

John Ogness

--
Dazuko Maintainer


_______________________________________________
Dazuko-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/dazuko-devel

[Lino Sanfilippo]

John Ogness wrote:

> On 2009-07-01, Lino Sanfilippo <[hidden email]> wrote:
>  
>> This patch ensures that generic_file_readonly_mmap() is only called
>> if the lower file actually supports mmap.
>>
>> If not the error code -ENODEV, which is used by the vfs to indicate
>> that mmap is not supported, is returned.
>>
>> +static int dazukofs_mmap(struct file *file, struct vm_area_struct *vm)
>> +{
>> + struct file *lower_file = get_lower_file(file);
>> +
>> + /* if lower fs does not support mmap, we dont call generic_mmap(), since
>> + * this would result in calling lower readpage(), which might not be defined
>> + * by lower fs, since mmap is not supported */
>> + if (!lower_file->f_op || !lower_file->f_op->mmap)
>> + return -ENODEV;
>> + return generic_file_mmap(file, vm);
>> +}
>>    
>
> Shouldn't it be:
>
>     if (!lower_file->f_op || !lower_file->f_op->mmap)
>             return -ENODEV;
>     return generic_file_readonly_mmap(file, vm);
>
> instead?
>
> John Ogness
>
>  

Oops, yes of course ;))

Geschäftsführender Gesellschafter: Tjark Auerbach
Sitz der Gesellschaft: Tettnang
Handelsregister: Amtsgericht Ulm, HRB 630992
ALLGEMEINE GESCHÄFTSBEDINGUNGEN
Es gelten unsere Allgemeinen Geschäftsbedingungen
(AGB). Sie finden sie in der jeweils gültigen Fassung
im Internet unter http://www.avira.de/agb
***************************************************


_______________________________________________
Dazuko-devel mailing list
[hidden email]
http://lists.nongnu.org/mailman/listinfo/dazuko-devel