* [PATCH V2] proc: fix ubsan warning in mem_lseek
@ 2019-09-02 6:57 yangerkun
2019-09-06 5:48 ` yangerkun
0 siblings, 1 reply; 5+ messages in thread
From: yangerkun @ 2019-09-02 6:57 UTC (permalink / raw)
To: viro, adobriyan; +Cc: linux-fsdevel, zhengbin13, yi.zhang, houtao1, yangerkun
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),
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@huawei.com>
---
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,
--
2.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH V2] proc: fix ubsan warning in mem_lseek
2019-09-02 6:57 [PATCH V2] proc: fix ubsan warning in mem_lseek yangerkun
@ 2019-09-06 5:48 ` yangerkun
2019-09-17 6:09 ` yangerkun
0 siblings, 1 reply; 5+ messages in thread
From: yangerkun @ 2019-09-06 5:48 UTC (permalink / raw)
To: viro, adobriyan; +Cc: linux-fsdevel, zhengbin13, yi.zhang, houtao1
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),
> 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@huawei.com>
> ---
> 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,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] proc: fix ubsan warning in mem_lseek
2019-09-06 5:48 ` yangerkun
@ 2019-09-17 6:09 ` yangerkun
2019-09-25 19:20 ` Alexey Dobriyan
0 siblings, 1 reply; 5+ messages in thread
From: yangerkun @ 2019-09-17 6:09 UTC (permalink / raw)
To: viro, adobriyan; +Cc: linux-fsdevel, zhengbin13, yi.zhang, houtao1
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),
>>
>> 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@huawei.com>
>> ---
>> 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,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] proc: fix ubsan warning in mem_lseek
2019-09-17 6:09 ` yangerkun
@ 2019-09-25 19:20 ` Alexey Dobriyan
2019-09-26 1:29 ` yangerkun
0 siblings, 1 reply; 5+ messages in thread
From: Alexey Dobriyan @ 2019-09-25 19:20 UTC (permalink / raw)
To: yangerkun; +Cc: viro, linux-fsdevel, zhengbin13, yi.zhang, houtao1
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.
> >> 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@huawei.com>
> >> ---
> >> 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,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] proc: fix ubsan warning in mem_lseek
2019-09-25 19:20 ` Alexey Dobriyan
@ 2019-09-26 1:29 ` yangerkun
0 siblings, 0 replies; 5+ messages in thread
From: yangerkun @ 2019-09-26 1:29 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: viro, linux-fsdevel, zhengbin13, yi.zhang, houtao1
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@huawei.com>
>>>> ---
>>>> 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,
>>
>
> .
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-26 1:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-02 6:57 [PATCH V2] proc: fix ubsan warning in mem_lseek yangerkun
2019-09-06 5:48 ` yangerkun
2019-09-17 6:09 ` yangerkun
2019-09-25 19:20 ` Alexey Dobriyan
2019-09-26 1:29 ` yangerkun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.