diff --git a/targets/ARCH/EXMIMO/DRIVER/eurecom/fileops.c b/targets/ARCH/EXMIMO/DRIVER/eurecom/fileops.c index 4427f4c8af49fc9d65461b0980d28218e5b31d03..ecedb562d7632fbf1d6df16086bd226d24173a71 100755 --- a/targets/ARCH/EXMIMO/DRIVER/eurecom/fileops.c +++ b/targets/ARCH/EXMIMO/DRIVER/eurecom/fileops.c @@ -179,6 +179,8 @@ int openair_device_ioctl(struct inode *inode,struct file *filp, unsigned int cmd int tmp; + unsigned int user_args[4]; + static unsigned int update_firmware_command; static unsigned int update_firmware_address; static unsigned int update_firmware_length; @@ -328,15 +330,38 @@ int openair_device_ioctl(struct inode *inode,struct file *filp, unsigned int cmd acknowledge with no limit */ #define MAX_IOCTL_ACK_CNT 500 - update_firmware_command = *((unsigned int*)arg); + /* 'arg' is an unsigned long and is supposed to be big enough + * to hold an address - this hypothesis is okay for i386 and x86_64 + * maybe not somewhere else + * this will probably fail with older kernels + * (works with 3.17 on x86_64) + */ + tmp = copy_from_user(&user_args, (void*)arg, 4*sizeof(unsigned int)); + if (tmp) { + printk("[openair][IOCTL] UPDATE_FIRMWARE: error copying parameters to kernel-space (%d bytes uncopied).\n", tmp); + return -1; + } + + update_firmware_command = user_args[0]; switch (update_firmware_command) { case UPDATE_FIRMWARE_TRANSFER_BLOCK: - update_firmware_address = ((unsigned int*)arg)[1]; - update_firmware_length = ((unsigned int*)arg)[2]; + update_firmware_address = user_args[1]; + update_firmware_length = user_args[2]; + + /* This is totally wrong on x86_64: a pointer is 64 bits and + * unsigned int is 32 bits. The userspace program has to ensure + * that its buffer address fits into 32 bits. + * If it proves a too strong requirement, we may change things + * in the future. + * The compiler emits a warning here. Do not remove this warning! + * This is to clearly remember this problem. + * If you require the compilation to work with zero warning, + * consider this one as an exception and find a proper workaround. + */ + update_firmware_ubuffer = user_args[3]; - update_firmware_ubuffer = (unsigned int*)((unsigned int*)arg)[3]; update_firmware_kbuffer = (unsigned int*)kmalloc(update_firmware_length * 4 /* 4 because kmalloc expects bytes */, GFP_KERNEL); @@ -390,8 +415,8 @@ int openair_device_ioctl(struct inode *inode,struct file *filp, unsigned int cmd case UPDATE_FIRMWARE_CLEAR_BSS: - update_firmware_bss_address = ((unsigned int*)arg)[1]; - update_firmware_bss_size = ((unsigned int*)arg)[2]; + update_firmware_bss_address = user_args[1]; + update_firmware_bss_size = user_args[2]; sparc_tmp_0 = update_firmware_bss_address; sparc_tmp_1 = update_firmware_bss_size; @@ -411,8 +436,8 @@ int openair_device_ioctl(struct inode *inode,struct file *filp, unsigned int cmd case UPDATE_FIRMWARE_START_EXECUTION: - update_firmware_start_address = ((unsigned int*)arg)[1]; - update_firmware_stack_pointer = ((unsigned int*)arg)[2]; + update_firmware_start_address = user_args[1]; + update_firmware_stack_pointer = user_args[2]; sparc_tmp_0 = update_firmware_start_address; sparc_tmp_1 = update_firmware_stack_pointer; diff --git a/targets/ARCH/EXMIMO/USERSPACE/OAI_FW_INIT/updatefw.c b/targets/ARCH/EXMIMO/USERSPACE/OAI_FW_INIT/updatefw.c index 692742c7cf82e41f3df0c8e78d3f7387b46679f2..cd873725c0598e1b8b2129c0a281b7d0f1eb48c6 100644 --- a/targets/ARCH/EXMIMO/USERSPACE/OAI_FW_INIT/updatefw.c +++ b/targets/ARCH/EXMIMO/USERSPACE/OAI_FW_INIT/updatefw.c @@ -41,6 +41,7 @@ #include <getopt.h> #include <string.h> #include <unistd.h> +#include <stdint.h> #include <sys/ioctl.h> #include "openair_device.h" @@ -239,6 +240,18 @@ void find_and_transfer_section(char* section_name, unsigned int verboselevel) { /* Dynamically allocate a chunk of memory to store the section into. */ section_content = (char*)malloc(elf_Shdr.sh_size); + /* Fail if the address returned by malloc does not fit in 32 bits. + * The kernel driver gets this address as 32 bits and uses it to copy + * from userspace. If the address does not fit into 32 bits the kernel + * will copy garbage or fail to copy completely. + * To be reworked if things do not work on some setups. + */ + if (sizeof(char*) > 4 && (((uintptr_t)section_content)>>32)) { + fprintf(stderr, "FATAL ERROR: an internal serious problem has been encountered.\n"); + fprintf(stderr, "Contact the authors so as to solve this issue.\n"); + exit(-1); + } + if (!section_content) { fprintf(stderr, "Error: could not dynamically allocate %d bytes for section %s.\n", elf_Shdr.sh_size, SecNameStnTable + elf_Shdr.sh_name); @@ -281,6 +294,13 @@ void find_and_transfer_section(char* section_name, unsigned int verboselevel) { ioctl_params[0] = UPDATE_FIRMWARE_TRANSFER_BLOCK; ioctl_params[1] = elf_Shdr.sh_addr; ioctl_params[2] = elf_Shdr.sh_size / 4; + /* This is wrong on x86_64 because a pointer is 64 bits and + * an unsigned int is only 32 bits. + * The compiler emits a warning, but the test + * above on the value of section_content makes it work + * (hopefully). + * To be changed if things do not work. + */ ioctl_params[3] = (unsigned int)((unsigned int*)section_content); //invert4(ioctl_params[1]); //invert4(ioctl_params[2]);