Re: [PATCH V2] proc: fix ubsan warning in mem_lseek

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2019/9/26 3:20, Alexey Dobriyan wrote:
On Tue, Sep 17, 2019 at 02:09:38PM +0800, yangerkun wrote:
Ping again...

On 2019/9/6 13:48, yangerkun wrote:
Ping.

On 2019/9/2 14:57, yangerkun wrote:
UBSAN has reported a overflow with mem_lseek. And it's fine with
mem_open set file mode with FMODE_UNSIGNED_OFFSET(memory_lseek).
However, another file use mem_lseek do lseek can have not
FMODE_UNSIGNED_OFFSET(proc_kpagecount_operations/proc_pagemap_operations),

Why those files can't have FMODE_UNSIGNED_OFFSET?
All files have unsigned offsets by definition, it is just lseek(SEEK_SET)
reusing signed type is silly.

I am not sure i understand what you say completely...

What i have seen is that every filesystem should set FMODE_UNSIGNED_OFFSET in its own 'open' operations(like mem_open) once they should use negative loff_t.


fix it by checking overflow and FMODE_UNSIGNED_OFFSET.

==================================================================
UBSAN: Undefined behaviour in ../fs/proc/base.c:941:15
signed integer overflow:
4611686018427387904 + 4611686018427387904 cannot be represented in
type 'long long int'
CPU: 4 PID: 4762 Comm: syz-executor.1 Not tainted 4.4.189 #3
Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
Call trace:
[<ffffff90080a5f28>] dump_backtrace+0x0/0x590
arch/arm64/kernel/traps.c:91
[<ffffff90080a64f0>] show_stack+0x38/0x60 arch/arm64/kernel/traps.c:234
[<ffffff9008986a34>] __dump_stack lib/dump_stack.c:15 [inline]
[<ffffff9008986a34>] dump_stack+0x128/0x184 lib/dump_stack.c:51
[<ffffff9008a2d120>] ubsan_epilogue+0x34/0x9c lib/ubsan.c:166
[<ffffff9008a2d8b8>] handle_overflow+0x228/0x280 lib/ubsan.c:197
[<ffffff9008a2da2c>] __ubsan_handle_add_overflow+0x4c/0x68
lib/ubsan.c:204
[<ffffff900862b9f4>] mem_lseek+0x12c/0x130 fs/proc/base.c:941
[<ffffff90084ef78c>] vfs_llseek fs/read_write.c:260 [inline]
[<ffffff90084ef78c>] SYSC_lseek fs/read_write.c:285 [inline]
[<ffffff90084ef78c>] SyS_lseek+0x164/0x1f0 fs/read_write.c:276
[<ffffff9008093c80>] el0_svc_naked+0x30/0x34
==================================================================

Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
---
   fs/proc/base.c     | 32 ++++++++++++++++++++++++--------
   fs/read_write.c    |  5 -----
   include/linux/fs.h |  5 +++++
   3 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index ebea950..a6c701b 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -882,18 +882,34 @@ static ssize_t mem_write(struct file *file,
const char __user *buf,
     loff_t mem_lseek(struct file *file, loff_t offset, int orig)
   {
+    loff_t ret = 0;
+
+    spin_lock(&file->f_lock);
       switch (orig) {
-    case 0:
-        file->f_pos = offset;
-        break;
-    case 1:
-        file->f_pos += offset;
+    case SEEK_CUR:
+        offset += file->f_pos;
+        /* fall through */
+    case SEEK_SET:
+        /* to avoid userland mistaking f_pos=-9 as -EBADF=-9 */
+        if ((unsigned long long)offset >= -MAX_ERRNO)
+            ret = -EOVERFLOW;
           break;
       default:
-        return -EINVAL;
+        ret = -EINVAL;
+    }
+
+    if (!ret) {
+        if (offset < 0 && !(unsigned_offsets(file))) {
+            ret = -EINVAL;
+        } else {
+            file->f_pos = offset;
+            ret = file->f_pos;
+            force_successful_syscall_return();
+        }
       }
-    force_successful_syscall_return();
-    return file->f_pos;
+
+    spin_unlock(&file->f_lock);
+    return ret;
   }
     static int mem_release(struct inode *inode, struct file *file)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5bbf587..961966e 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -34,11 +34,6 @@ const struct file_operations generic_ro_fops = {
     EXPORT_SYMBOL(generic_ro_fops);
   -static inline bool unsigned_offsets(struct file *file)
-{
-    return file->f_mode & FMODE_UNSIGNED_OFFSET;
-}
-
   /**
    * vfs_setpos - update the file offset for lseek
    * @file:    file structure in question
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 997a530..e5edbc9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3074,6 +3074,11 @@ extern void
   file_ra_state_init(struct file_ra_state *ra, struct address_space
*mapping);
   extern loff_t noop_llseek(struct file *file, loff_t offset, int
whence);
   extern loff_t no_llseek(struct file *file, loff_t offset, int whence);
+static inline bool unsigned_offsets(struct file *file)
+{
+    return file->f_mode & FMODE_UNSIGNED_OFFSET;
+}
+
   extern loff_t vfs_setpos(struct file *file, loff_t offset, loff_t
maxsize);
   extern loff_t generic_file_llseek(struct file *file, loff_t offset,
int whence);
   extern loff_t generic_file_llseek_size(struct file *file, loff_t
offset,


.





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux