diff mbox series

[v3,1/1] vsprintf: protect kernel from panic due to non-canonical pointer dereference

Message ID 20221019193431.2923462-2-jane.chu@oracle.com
State New
Headers
Series vsprintf: check non-canonical pointer by kern_addr_valid() |

Commit Message

Jane Chu Oct. 19, 2022, 7:34 p.m. UTC 
  Having stepped on a local kernel bug where reading sysfs has led to
out-of-bound pointer dereference by vsprintf() which led to GPF panic.
And the reason for GPF is that the OOB pointer was turned to a
non-canonical address such as 0x7665645f63616465.

vsprintf() already has this line of defense
	if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
                return "(efault)";
Since a non-canonical pointer can be detected by kern_addr_valid()
on architectures that present VM holes as well as meaningful
implementation of kern_addr_valid() that detects the non-canonical
addresses, this patch addes a check on non-canonical string pointer by
kern_addr_valid() and display "(efault)" to alert user that something
is wrong instead of unecessarily panic the server.

On the other hand, if the non-canonical string pointer is dereferenced
else where in the kernel, by virtue of being non-canonical, a crash
is expected to be immediate.

Signed-off-by: Jane Chu <jane.chu@oracle.com>
---
 lib/vsprintf.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

kernel test robot Oct. 20, 2022, 11:41 a.m. UTC | #1 
Hi Jane,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.1-rc1]
[cannot apply to next-20221020]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jane-Chu/vsprintf-check-non-canonical-pointer-by-kern_addr_valid/20221020-103535
patch link:    https://lore.kernel.org/r/20221019193431.2923462-2-jane.chu%40oracle.com
patch subject: [PATCH v3 1/1] vsprintf: protect kernel from panic due to non-canonical pointer dereference
config: x86_64-rhel-8.3-func
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/7da79322bb256f65be136ef3ca3d557e42da8ffe
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Jane-Chu/vsprintf-check-non-canonical-pointer-by-kern_addr_valid/20221020-103535
        git checkout 7da79322bb256f65be136ef3ca3d557e42da8ffe
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   lib/vsprintf.c: In function 'check_pointer_msg':
>> lib/vsprintf.c:701:14: error: implicit declaration of function 'kern_addr_valid'; did you mean 'virt_addr_valid'? [-Werror=implicit-function-declaration]
     701 |         if (!kern_addr_valid((unsigned long)ptr))
         |              ^~~~~~~~~~~~~~~
         |              virt_addr_valid
   lib/vsprintf.c: In function 'va_format':
   lib/vsprintf.c:1688:9: warning: function 'va_format' might be a candidate for 'gnu_printf' format attribute [-Wsuggest-attribute=format]
    1688 |         buf += vsnprintf(buf, end > buf ? end - buf : 0, va_fmt->fmt, va);
         |         ^~~
   cc1: some warnings being treated as errors


vim +701 lib/vsprintf.c

   687	
   688	/*
   689	 * Do not call any complex external code here. Nested printk()/vsprintf()
   690	 * might cause infinite loops. Failures might break printk() and would
   691	 * be hard to debug.
   692	 */
   693	static const char *check_pointer_msg(const void *ptr)
   694	{
   695		if (!ptr)
   696			return "(null)";
   697	
   698		if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
   699			return "(efault)";
   700	
 > 701		if (!kern_addr_valid((unsigned long)ptr))
   702			return "(efault)";
   703	
   704		return NULL;
   705	}
   706
diff mbox series

Patch

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index c414a8d9f1ea..b38c12ef1e45 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -698,6 +698,9 @@  static const char *check_pointer_msg(const void *ptr)
 	if ((unsigned long)ptr < PAGE_SIZE || IS_ERR_VALUE(ptr))
 		return "(efault)";
 
+	if (!kern_addr_valid((unsigned long)ptr))
+		return "(efault)";
+
 	return NULL;
 }