Commit eca6f534e61919b28fb21aafbd1c2983deae75be

Authored by Vegard Nossum
Committed by al
1 parent 6d729e44a5

fs: fix overflow in sys_mount() for in-kernel calls

sys_mount() reads/copies a whole page for its "type" parameter.  When
do_mount_root() passes a kernel address that points to an object which is
smaller than a whole page, copy_mount_options() will happily go past this
memory object, possibly dereferencing "wild" pointers that could be in any
state (hence the kmemcheck warning, which shows that parts of the next
page are not even allocated).

(The likelihood of something going wrong here is pretty low -- first of
all this only applies to kernel calls to sys_mount(), which are mostly
found in the boot code.  Secondly, I guess if the page was not mapped,
exact_copy_from_user() _would_ in fact handle it correctly because of its
access_ok(), etc.  checks.)

But it is much nicer to avoid the dubious reads altogether, by stopping as
soon as we find a NUL byte.  Is there a good reason why we can't do
something like this, using the already existing strndup_from_user()?

[akpm@linux-foundation.org: make copy_mount_string() static]
[AV: fix compat mount breakage, which involves undoing akpm's change above]

Reported-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: al <al@dizzy.pdmi.ras.ru>

Showing 3 changed files with 60 additions and 42 deletions Side-by-side Diff

... ... @@ -768,13 +768,13 @@
768 768 char __user * type, unsigned long flags,
769 769 void __user * data)
770 770 {
771   - unsigned long type_page;
  771 + char *kernel_type;
772 772 unsigned long data_page;
773   - unsigned long dev_page;
  773 + char *kernel_dev;
774 774 char *dir_page;
775 775 int retval;
776 776  
777   - retval = copy_mount_options (type, &type_page);
  777 + retval = copy_mount_string(type, &kernel_type);
778 778 if (retval < 0)
779 779 goto out;
780 780  
781 781  
782 782  
783 783  
784 784  
785 785  
786 786  
787 787  
... ... @@ -783,38 +783,38 @@
783 783 if (IS_ERR(dir_page))
784 784 goto out1;
785 785  
786   - retval = copy_mount_options (dev_name, &dev_page);
  786 + retval = copy_mount_string(dev_name, &kernel_dev);
787 787 if (retval < 0)
788 788 goto out2;
789 789  
790   - retval = copy_mount_options (data, &data_page);
  790 + retval = copy_mount_options(data, &data_page);
791 791 if (retval < 0)
792 792 goto out3;
793 793  
794 794 retval = -EINVAL;
795 795  
796   - if (type_page && data_page) {
797   - if (!strcmp((char *)type_page, SMBFS_NAME)) {
  796 + if (kernel_type && data_page) {
  797 + if (!strcmp(kernel_type, SMBFS_NAME)) {
798 798 do_smb_super_data_conv((void *)data_page);
799   - } else if (!strcmp((char *)type_page, NCPFS_NAME)) {
  799 + } else if (!strcmp(kernel_type, NCPFS_NAME)) {
800 800 do_ncp_super_data_conv((void *)data_page);
801   - } else if (!strcmp((char *)type_page, NFS4_NAME)) {
  801 + } else if (!strcmp(kernel_type, NFS4_NAME)) {
802 802 if (do_nfs4_super_data_conv((void *) data_page))
803 803 goto out4;
804 804 }
805 805 }
806 806  
807   - retval = do_mount((char*)dev_page, dir_page, (char*)type_page,
  807 + retval = do_mount(kernel_dev, dir_page, kernel_type,
808 808 flags, (void*)data_page);
809 809  
810 810 out4:
811 811 free_page(data_page);
812 812 out3:
813   - free_page(dev_page);
  813 + kfree(kernel_dev);
814 814 out2:
815 815 putname(dir_page);
816 816 out1:
817   - free_page(type_page);
  817 + kfree(kernel_type);
818 818 out:
819 819 return retval;
820 820 }
... ... @@ -57,6 +57,7 @@
57 57 * namespace.c
58 58 */
59 59 extern int copy_mount_options(const void __user *, unsigned long *);
  60 +extern int copy_mount_string(const void __user *, char **);
60 61  
61 62 extern void free_vfsmnt(struct vfsmount *);
62 63 extern struct vfsmount *alloc_vfsmnt(const char *);
... ... @@ -1640,7 +1640,7 @@
1640 1640 {
1641 1641 struct vfsmount *mnt;
1642 1642  
1643   - if (!type || !memchr(type, 0, PAGE_SIZE))
  1643 + if (!type)
1644 1644 return -EINVAL;
1645 1645  
1646 1646 /* we need capabilities... */
... ... @@ -1871,6 +1871,23 @@
1871 1871 return 0;
1872 1872 }
1873 1873  
  1874 +int copy_mount_string(const void __user *data, char **where)
  1875 +{
  1876 + char *tmp;
  1877 +
  1878 + if (!data) {
  1879 + *where = NULL;
  1880 + return 0;
  1881 + }
  1882 +
  1883 + tmp = strndup_user(data, PAGE_SIZE);
  1884 + if (IS_ERR(tmp))
  1885 + return PTR_ERR(tmp);
  1886 +
  1887 + *where = tmp;
  1888 + return 0;
  1889 +}
  1890 +
1874 1891 /*
1875 1892 * Flags is a 32-bit value that allows up to 31 non-fs dependent flags to
1876 1893 * be given to the mount() call (ie: read-only, no-dev, no-suid etc).
... ... @@ -1900,8 +1917,6 @@
1900 1917  
1901 1918 if (!dir_name || !*dir_name || !memchr(dir_name, 0, PAGE_SIZE))
1902 1919 return -EINVAL;
1903   - if (dev_name && !memchr(dev_name, 0, PAGE_SIZE))
1904   - return -EINVAL;
1905 1920  
1906 1921 if (data_page)
1907 1922 ((char *)data_page)[PAGE_SIZE - 1] = 0;
1908 1923  
1909 1924  
1910 1925  
1911 1926  
1912 1927  
1913 1928  
1914 1929  
... ... @@ -2070,40 +2085,42 @@
2070 2085 SYSCALL_DEFINE5(mount, char __user *, dev_name, char __user *, dir_name,
2071 2086 char __user *, type, unsigned long, flags, void __user *, data)
2072 2087 {
2073   - int retval;
  2088 + int ret;
  2089 + char *kernel_type;
  2090 + char *kernel_dir;
  2091 + char *kernel_dev;
2074 2092 unsigned long data_page;
2075   - unsigned long type_page;
2076   - unsigned long dev_page;
2077   - char *dir_page;
2078 2093  
2079   - retval = copy_mount_options(type, &type_page);
2080   - if (retval < 0)
2081   - return retval;
  2094 + ret = copy_mount_string(type, &kernel_type);
  2095 + if (ret < 0)
  2096 + goto out_type;
2082 2097  
2083   - dir_page = getname(dir_name);
2084   - retval = PTR_ERR(dir_page);
2085   - if (IS_ERR(dir_page))
2086   - goto out1;
  2098 + kernel_dir = getname(dir_name);
  2099 + if (IS_ERR(kernel_dir)) {
  2100 + ret = PTR_ERR(kernel_dir);
  2101 + goto out_dir;
  2102 + }
2087 2103  
2088   - retval = copy_mount_options(dev_name, &dev_page);
2089   - if (retval < 0)
2090   - goto out2;
  2104 + ret = copy_mount_string(dev_name, &kernel_dev);
  2105 + if (ret < 0)
  2106 + goto out_dev;
2091 2107  
2092   - retval = copy_mount_options(data, &data_page);
2093   - if (retval < 0)
2094   - goto out3;
  2108 + ret = copy_mount_options(data, &data_page);
  2109 + if (ret < 0)
  2110 + goto out_data;
2095 2111  
2096   - retval = do_mount((char *)dev_page, dir_page, (char *)type_page,
2097   - flags, (void *)data_page);
2098   - free_page(data_page);
  2112 + ret = do_mount(kernel_dev, kernel_dir, kernel_type, flags,
  2113 + (void *) data_page);
2099 2114  
2100   -out3:
2101   - free_page(dev_page);
2102   -out2:
2103   - putname(dir_page);
2104   -out1:
2105   - free_page(type_page);
2106   - return retval;
  2115 + free_page(data_page);
  2116 +out_data:
  2117 + kfree(kernel_dev);
  2118 +out_dev:
  2119 + putname(kernel_dir);
  2120 +out_dir:
  2121 + kfree(kernel_type);
  2122 +out_type:
  2123 + return ret;
2107 2124 }
2108 2125  
2109 2126 /*