maccess: make get_kernel_nofault() check for minimal type compatibility

Now that we've renamed probe_kernel_address() to get_kernel_nofault()
and made it look and behave more in line with get_user(), some of the
subtle type behavior differences end up being more obvious and possibly
dangerous.

When you do

        get_user(val, user_ptr);

the type of the access comes from the "user_ptr" part, and the above
basically acts as

        val = *user_ptr;

by design (except, of course, for the fact that the actual dereference
is done with a user access).

Note how in the above case, the type of the end result comes from the
pointer argument, and then the value is cast to the type of 'val' as
part of the assignment.

So the type of the pointer is ultimately the more important type both
for the access itself.

But 'get_kernel_nofault()' may now _look_ similar, but it behaves very
differently.  When you do

        get_kernel_nofault(val, kernel_ptr);

it behaves like

        val = *(typeof(val) *)kernel_ptr;

except, of course, for the fact that the actual dereference is done with
exception handling so that a faulting access is suppressed and returned
as the error code.

But note how different the casting behavior of the two superficially
similar accesses are: one does the actual access in the size of the type
the pointer points to, while the other does the access in the size of
the target, and ignores the pointer type entirely.

Actually changing get_kernel_nofault() to act like get_user() is almost
certainly the right thing to do eventually, but in the meantime this
patch adds logit to at least verify that the pointer type is compatible
with the type of the result.

In many cases, this involves just casting the pointer to 'void *' to
make it obvious that the type of the pointer is not the important part.
It's not how 'get_user()' acts, but at least the behavioral difference
is now obvious and explicit.

Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index 49ce15c3612d6ee111a85893966de4d2c54c4d3d..17d5a785df28b10ee5714246d3ed342192c25e75 100644 (file)
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -396,7 +396,7 @@ int is_valid_bugaddr(unsigned long pc)
        u32 insn = __opcode_to_mem_arm(BUG_INSTR_VALUE);
 #endif
 
-       if (get_kernel_nofault(bkpt, (unsigned *)pc))
+       if (get_kernel_nofault(bkpt, (void *)pc))
                return 0;
 
        return bkpt == insn;

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index ad4fc06e5f4bb1c466f8709b58a91fe7e82d8a8b..3a033d2008b3c5d87e7741c667b84d13769d1790 100644 (file)
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -35,7 +35,7 @@ static inline void *dereference_function_descriptor(void *ptr)
        struct fdesc *desc = ptr;
        void *p;
 
-       if (!get_kernel_nofault(p, &desc->ip))
+       if (!get_kernel_nofault(p, (void *)&desc->ip))
                ptr = p;
        return ptr;
 }

diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 6c435dbccca0e9a41efaa5a92476ca040b68a0bd..b7abb12edd3a4bf8b2b49cc38712a8e9bcf50fa7 100644 (file)
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -293,7 +293,7 @@ void *dereference_function_descriptor(void *ptr)
        Elf64_Fdesc *desc = ptr;
        void *p;
 
-       if (!get_kernel_nofault(p, &desc->addr))
+       if (!get_kernel_nofault(p, (void *)&desc->addr))
                ptr = p;
        return ptr;
 }

diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index bd311616fca8e8232d67f3ffea99af55242936cc..324d7b298ec344aa76989cfd72f635662bc02f05 100644 (file)
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -85,7 +85,7 @@ static inline void *dereference_function_descriptor(void *ptr)
        struct ppc64_opd_entry *desc = ptr;
        void *p;
 
-       if (!get_kernel_nofault(p, &desc->funcaddr))
+       if (!get_kernel_nofault(p, (void *)&desc->funcaddr))
                ptr = p;
        return ptr;
 }

diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index e14a1862a3caee9763c3838a07065d51ba5fe618..409080208a6c4ae104e01e341ffebd6082585024 100644 (file)
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -421,7 +421,7 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
        unsigned int instr;
        struct ppc_inst *addr = (struct ppc_inst *)bpt->bpt_addr;
 
-       err = get_kernel_nofault(instr, addr);
+       err = get_kernel_nofault(instr, (unsigned *) addr);
        if (err)
                return err;
 

diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index 65b0dd2bf25cc68909c16dae067c5e28e0faed86..9e1def3744f2202349061a48a3a6a8c42c77ea22 100644 (file)
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -94,7 +94,7 @@ static bool match_id(struct pci_dev *pdev, unsigned short vendor, unsigned short
 }
 
 static bool probe_list(struct pci_dev *pdev, unsigned short vendor,
-                      const unsigned char *rom_list)
+                      const void *rom_list)
 {
        unsigned short device;
 
@@ -119,7 +119,7 @@ static struct resource *find_oprom(struct pci_dev *pdev)
        for (i = 0; i < ARRAY_SIZE(adapter_rom_resources); i++) {
                struct resource *res = &adapter_rom_resources[i];
                unsigned short offset, vendor, device, list, rev;
-               const unsigned char *rom;
+               const void *rom;
 
                if (res->end == 0)
                        break;

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index a508a3c088794a745d666ab2f46abd4697073997..0a76ddc07d59704b1c24ac5ef78bc2c5b05ba50b 100644 (file)
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -324,8 +324,10 @@ long strnlen_user_nofault(const void __user *unsafe_addr, long count);
  *
  * Returns 0 on success, or -EFAULT.
  */
-#define get_kernel_nofault(val, ptr)           \
-       copy_from_kernel_nofault(&(val), (ptr), sizeof(val))
+#define get_kernel_nofault(val, ptr) ({                                \
+       const typeof(val) *__gk_ptr = (ptr);                    \
+       copy_from_kernel_nofault(&(val), __gk_ptr, sizeof(val));\
+})
 
 #ifndef user_access_begin
 #define user_access_begin(ptr,len) access_ok(ptr, len)