* [PATCH 1/3] dlm: fix invalid free
2018-11-02 20:18 [PATCH 0/3] dlm: fix various incorrect behaviors Tycho Andersen
@ 2018-11-02 20:18 ` Tycho Andersen
2018-11-02 20:18 ` [PATCH 2/3] dlm: don't allow zero length names Tycho Andersen
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Tycho Andersen @ 2018-11-02 20:18 UTC (permalink / raw)
To: Christine Caulfield, David Teigland
Cc: cluster-devel, linux-kernel, Tycho Andersen
dlm_config_nodes() does not allocate nodes on failure, so we should not
free() nodes when it fails.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
---
fs/dlm/member.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/dlm/member.c b/fs/dlm/member.c
index 3fda3832cf6a..cad6d85911a8 100644
--- a/fs/dlm/member.c
+++ b/fs/dlm/member.c
@@ -680,7 +680,7 @@ int dlm_ls_start(struct dlm_ls *ls)
error = dlm_config_nodes(ls->ls_name, &nodes, &count);
if (error < 0)
- goto fail;
+ goto fail_rv;
spin_lock(&ls->ls_recover_lock);
@@ -712,8 +712,9 @@ int dlm_ls_start(struct dlm_ls *ls)
return 0;
fail:
- kfree(rv);
kfree(nodes);
+ fail_rv:
+ kfree(rv);
return error;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] dlm: don't allow zero length names
2018-11-02 20:18 [PATCH 0/3] dlm: fix various incorrect behaviors Tycho Andersen
2018-11-02 20:18 ` [PATCH 1/3] dlm: fix invalid free Tycho Andersen
@ 2018-11-02 20:18 ` Tycho Andersen
2018-11-02 20:18 ` [PATCH 3/3] dlm: don't leak kernel pointer to userspace Tycho Andersen
2018-11-07 22:20 ` [PATCH 0/3] dlm: fix various incorrect behaviors David Teigland
3 siblings, 0 replies; 7+ messages in thread
From: Tycho Andersen @ 2018-11-02 20:18 UTC (permalink / raw)
To: Christine Caulfield, David Teigland
Cc: cluster-devel, linux-kernel, Tycho Andersen
kobject doesn't like zero length object names, so let's test for that.
Nov 2 19:48:14 dlm kernel: [ 20.548713] ------------[ cut here ]------------
Nov 2 19:48:14 dlm kernel: [ 20.548717] kobject: ((____ptrval____)): attempted to be registered with empty name!
Nov 2 19:48:14 dlm kernel: [ 20.548747] WARNING: CPU: 2 PID: 1368 at lib/kobject.c:228 kobject_add_internal+0x689/0x800
Nov 2 19:48:14 dlm kernel: [ 20.548750] Modules linked in: fuse
Nov 2 19:48:14 dlm kernel: [ 20.548758] CPU: 2 PID: 1368 Comm: dlm Not tainted 4.19.0+ #36
Nov 2 19:48:14 dlm kernel: [ 20.548762] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1ubuntu1 04/01/2014
Nov 2 19:48:14 dlm kernel: [ 20.548767] RIP: 0010:kobject_add_internal+0x689/0x800
Nov 2 19:48:14 dlm kernel: [ 20.548771] Code: ff 48 89 df e8 d8 ec fd fe e9 fb f9 ff ff 48 89 df e8 fb ec fd fe e9 7b fa ff ff 4c 89 fe 48 c7 c7 00 e6 ed 82 e8 97 a8 b4 fe <0f> 0b bb ea ff ff ff e9 fb fb ff ff bb fe ff ff ff e9 f1 fb ff ff
Nov 2 19:48:14 dlm kernel: [ 20.548775] RSP: 0018:ffff88011441f988 EFLAGS: 00010282
Nov 2 19:48:14 dlm kernel: [ 20.548779] RAX: 0000000000000000 RBX: ffff880113d36600 RCX: ffffffff813c4a70
Nov 2 19:48:14 dlm kernel: [ 20.548783] RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88011612e770
Nov 2 19:48:14 dlm kernel: [ 20.548786] RBP: ffff88011441faa8 R08: ffffed0022c25cef R09: ffffed0022c25cef
Nov 2 19:48:14 dlm kernel: [ 20.548790] R10: 0000000000000001 R11: ffffed0022c25cee R12: 1ffff10022883f3d
Nov 2 19:48:14 dlm kernel: [ 20.548793] R13: ffffffff82d005c0 R14: 0000000000000000 R15: ffff880112d78040
Nov 2 19:48:14 dlm kernel: [ 20.548797] FS: 00007f441ba404c0(0000) GS:ffff880116100000(0000) knlGS:0000000000000000
Nov 2 19:48:14 dlm kernel: [ 20.548801] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
Nov 2 19:48:14 dlm kernel: [ 20.548804] CR2: 00007f441b53ebb0 CR3: 00000001103c0000 CR4: 00000000000006e0
Nov 2 19:48:14 dlm kernel: [ 20.548810] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
Nov 2 19:48:14 dlm kernel: [ 20.548814] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Nov 2 19:48:14 dlm kernel: [ 20.548817] Call Trace:
Nov 2 19:48:14 dlm kernel: [ 20.548833] kobject_init_and_add+0xe4/0x140
Nov 2 19:48:14 dlm kernel: [ 20.548840] ? kset_create_and_add+0x150/0x150
Nov 2 19:48:14 dlm kernel: [ 20.548845] ? printk+0x97/0xbe
Nov 2 19:48:14 dlm kernel: [ 20.548859] ? _raw_spin_unlock_irqrestore+0x2d/0x50
Nov 2 19:48:14 dlm kernel: [ 20.548874] dlm_new_lockspace+0x1766/0x1df0
Nov 2 19:48:14 dlm kernel: [ 20.548893] ? dlm_put_lockspace+0x50/0x50
Nov 2 19:48:14 dlm kernel: [ 20.548901] ? wait_woken+0x260/0x260
Nov 2 19:48:14 dlm kernel: [ 20.548913] ? security_capable+0x4f/0x90
Nov 2 19:48:14 dlm kernel: [ 20.548924] device_write+0xb57/0x1bb0
Nov 2 19:48:14 dlm kernel: [ 20.548935] ? device_read+0xa60/0xa60
Nov 2 19:48:14 dlm kernel: [ 20.548941] ? __kasan_slab_free+0x12e/0x180
Nov 2 19:48:14 dlm kernel: [ 20.548945] ? kmem_cache_free+0xb9/0x2e0
Nov 2 19:48:14 dlm kernel: [ 20.548949] ? do_sys_open+0x182/0x340
Nov 2 19:48:14 dlm kernel: [ 20.548954] ? do_syscall_64+0x90/0x3e0
Nov 2 19:48:14 dlm kernel: [ 20.548959] ? entry_SYSCALL_64_after_hwframe+0x49/0xbe
Nov 2 19:48:14 dlm kernel: [ 20.548967] ? debug_show_all_locks+0x2d0/0x2d0
Nov 2 19:48:14 dlm kernel: [ 20.548979] __vfs_write+0xd3/0x5c0
Nov 2 19:48:14 dlm kernel: [ 20.548984] ? kernel_read+0x130/0x130
Nov 2 19:48:14 dlm kernel: [ 20.548987] ? find_held_lock+0x32/0x1c0
Nov 2 19:48:14 dlm kernel: [ 20.548993] ? find_held_lock+0x32/0x1c0
Nov 2 19:48:14 dlm kernel: [ 20.549001] ? quarantine_put+0x8e/0x150
Nov 2 19:48:14 dlm kernel: [ 20.549009] ? do_sys_open+0x182/0x340
Nov 2 19:48:14 dlm kernel: [ 20.549015] vfs_write+0x157/0x4a0
Nov 2 19:48:14 dlm kernel: [ 20.549021] ksys_write+0xb3/0x180
Nov 2 19:48:14 dlm kernel: [ 20.549026] ? __ia32_sys_read+0xa0/0xa0
Nov 2 19:48:14 dlm kernel: [ 20.549028] ? entry_SYSCALL_64_after_hwframe+0x3e/0xbe
Nov 2 19:48:14 dlm kernel: [ 20.549033] ? trace_hardirqs_off_caller+0x5b/0x160
Nov 2 19:48:14 dlm kernel: [ 20.549038] ? do_syscall_64+0x13/0x3e0
Nov 2 19:48:14 dlm kernel: [ 20.549044] do_syscall_64+0x90/0x3e0
Nov 2 19:48:14 dlm kernel: [ 20.549049] entry_SYSCALL_64_after_hwframe+0x49/0xbe
Nov 2 19:48:14 dlm kernel: [ 20.549052] RIP: 0033:0x7f441b53f154
Nov 2 19:48:14 dlm kernel: [ 20.549055] Code: 89 02 48 c7 c0 ff ff ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 8d 05 b1 07 2e 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 f3 c3 66 90 41 54 55 49 89 d4 53 48 89 f5
Nov 2 19:48:14 dlm kernel: [ 20.549057] RSP: 002b:00007ffefadc1e88 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
Nov 2 19:48:14 dlm kernel: [ 20.549061] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f441b53f154
Nov 2 19:48:14 dlm kernel: [ 20.549063] RDX: 000000000000007c RSI: 00007ffefadc1f30 RDI: 0000000000000003
Nov 2 19:48:14 dlm kernel: [ 20.549065] RBP: 00007ffefadc1fc0 R08: 0000000000000004 R09: 0000000000000000
Nov 2 19:48:14 dlm kernel: [ 20.549067] R10: 0000000000000000 R11: 0000000000000246 R12: 000055817bf3f760
Nov 2 19:48:14 dlm kernel: [ 20.549069] R13: 00007ffefadc20a0 R14: 0000000000000000 R15: 0000000000000000
Nov 2 19:48:14 dlm kernel: [ 20.549082] irq event stamp: 3866
Nov 2 19:48:14 dlm kernel: [ 20.549086] hardirqs last enabled at (3865): [<ffffffff8124d00d>] console_unlock+0x55d/0xa90
Nov 2 19:48:14 dlm kernel: [ 20.549089] hardirqs last disabled at (3866): [<ffffffff8100364b>] trace_hardirqs_off_thunk+0x1a/0x1c
Nov 2 19:48:14 dlm kernel: [ 20.549092] softirqs last enabled at (3838): [<ffffffff82a004a9>] __do_softirq+0x4a9/0x8fb
Nov 2 19:48:14 dlm kernel: [ 20.549095] softirqs last disabled at (3761): [<ffffffff8112c9c1>] irq_exit+0x1a1/0x1d0
Nov 2 19:48:14 dlm kernel: [ 20.549097] ---[ end trace 56e315ac4368655c ]---
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
---
fs/dlm/lockspace.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dlm/lockspace.c b/fs/dlm/lockspace.c
index 5ba94be006ee..7b9d47fe7a34 100644
--- a/fs/dlm/lockspace.c
+++ b/fs/dlm/lockspace.c
@@ -431,7 +431,7 @@ static int new_lockspace(const char *name, const char *cluster,
int do_unreg = 0;
int namelen = strlen(name);
- if (namelen > DLM_LOCKSPACE_LEN)
+ if (namelen > DLM_LOCKSPACE_LEN || namelen == 0)
return -EINVAL;
if (!lvblen || (lvblen % 8))
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] dlm: don't leak kernel pointer to userspace
2018-11-02 20:18 [PATCH 0/3] dlm: fix various incorrect behaviors Tycho Andersen
2018-11-02 20:18 ` [PATCH 1/3] dlm: fix invalid free Tycho Andersen
2018-11-02 20:18 ` [PATCH 2/3] dlm: don't allow zero length names Tycho Andersen
@ 2018-11-02 20:18 ` Tycho Andersen
2018-11-07 22:20 ` [PATCH 0/3] dlm: fix various incorrect behaviors David Teigland
3 siblings, 0 replies; 7+ messages in thread
From: Tycho Andersen @ 2018-11-02 20:18 UTC (permalink / raw)
To: Christine Caulfield, David Teigland
Cc: cluster-devel, linux-kernel, Tycho Andersen
In copy_result_to_user(), we first create a struct dlm_lock_result, which
contains a struct dlm_lksb, the last member of which is a pointer to the
lvb. Unfortunately, we copy the entire struct dlm_lksb to the result
struct, which is then copied to userspace at the end of the function,
leaking the contents of sb_lvbptr, which is a valid kernel pointer in some
cases (indeed, later in the same function the data it points to is copied
to userspace).
It is an error to leak kernel pointers to userspace, as it undermines KASLR
protections (see e.g. 65eea8edc31 ("floppy: Do not copy a kernel pointer to
user memory in FDGETPRM ioctl") for another example of this).
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
---
fs/dlm/user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index 2a669390cd7f..13f29409600b 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -702,7 +702,7 @@ static int copy_result_to_user(struct dlm_user_args *ua, int compat,
result.version[0] = DLM_DEVICE_VERSION_MAJOR;
result.version[1] = DLM_DEVICE_VERSION_MINOR;
result.version[2] = DLM_DEVICE_VERSION_PATCH;
- memcpy(&result.lksb, &ua->lksb, sizeof(struct dlm_lksb));
+ memcpy(&result.lksb, &ua->lksb, offsetof(struct dlm_lksb, sb_lvbptr));
result.user_lksb = ua->user_lksb;
/* FIXME: dlm1 provides for the user's bastparam/addr to not be updated
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] dlm: fix various incorrect behaviors
2018-11-02 20:18 [PATCH 0/3] dlm: fix various incorrect behaviors Tycho Andersen
` (2 preceding siblings ...)
2018-11-02 20:18 ` [PATCH 3/3] dlm: don't leak kernel pointer to userspace Tycho Andersen
@ 2018-11-07 22:20 ` David Teigland
2018-11-09 15:15 ` Tycho Andersen
3 siblings, 1 reply; 7+ messages in thread
From: David Teigland @ 2018-11-07 22:20 UTC (permalink / raw)
To: Tycho Andersen; +Cc: Christine Caulfield, cluster-devel, linux-kernel
On Fri, Nov 02, 2018 at 02:18:19PM -0600, Tycho Andersen wrote:
> Hi,
>
> here's a series to fix some bugs I noticed in the DLM. The third patch
> in the series and maybe the first should probably go to stable, assuming
> everyone agrees they're indeed bugs.
>
> Thanks,
>
> Tycho
>
> Tycho Andersen (3):
> dlm: fix invalid free
> dlm: don't allow zero length names
> dlm: don't leak kernel pointer to userspace
>
> fs/dlm/lockspace.c | 2 +-
> fs/dlm/member.c | 5 +++--
> fs/dlm/user.c | 2 +-
> 3 files changed, 5 insertions(+), 4 deletions(-)
I've pushed these to linux-dlm next.
Dave
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] dlm: fix various incorrect behaviors
2018-11-07 22:20 ` [PATCH 0/3] dlm: fix various incorrect behaviors David Teigland
@ 2018-11-09 15:15 ` Tycho Andersen
2018-11-15 18:24 ` Kees Cook
0 siblings, 1 reply; 7+ messages in thread
From: Tycho Andersen @ 2018-11-09 15:15 UTC (permalink / raw)
To: David Teigland; +Cc: Christine Caulfield, cluster-devel, linux-kernel
On Wed, Nov 07, 2018 at 04:20:42PM -0600, David Teigland wrote:
> On Fri, Nov 02, 2018 at 02:18:19PM -0600, Tycho Andersen wrote:
> > Hi,
> >
> > here's a series to fix some bugs I noticed in the DLM. The third patch
> > in the series and maybe the first should probably go to stable, assuming
> > everyone agrees they're indeed bugs.
> >
> > Thanks,
> >
> > Tycho
> >
> > Tycho Andersen (3):
> > dlm: fix invalid free
> > dlm: don't allow zero length names
> > dlm: don't leak kernel pointer to userspace
> >
> > fs/dlm/lockspace.c | 2 +-
> > fs/dlm/member.c | 5 +++--
> > fs/dlm/user.c | 2 +-
> > 3 files changed, 5 insertions(+), 4 deletions(-)
>
> I've pushed these to linux-dlm next.
Great, thanks! Should we send 1 and 3 to stable?
Tycho
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/3] dlm: fix various incorrect behaviors
2018-11-09 15:15 ` Tycho Andersen
@ 2018-11-15 18:24 ` Kees Cook
0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2018-11-15 18:24 UTC (permalink / raw)
To: Tycho Andersen; +Cc: David Teigland, Christine Caulfield, cluster-devel, LKML
On Fri, Nov 9, 2018 at 9:15 AM, Tycho Andersen <tycho@tycho.ws> wrote:
> On Wed, Nov 07, 2018 at 04:20:42PM -0600, David Teigland wrote:
>> On Fri, Nov 02, 2018 at 02:18:19PM -0600, Tycho Andersen wrote:
>> > Hi,
>> >
>> > here's a series to fix some bugs I noticed in the DLM. The third patch
>> > in the series and maybe the first should probably go to stable, assuming
>> > everyone agrees they're indeed bugs.
>> >
>> > Thanks,
>> >
>> > Tycho
>> >
>> > Tycho Andersen (3):
>> > dlm: fix invalid free
>> > dlm: don't allow zero length names
>> > dlm: don't leak kernel pointer to userspace
>> >
>> > fs/dlm/lockspace.c | 2 +-
>> > fs/dlm/member.c | 5 +++--
>> > fs/dlm/user.c | 2 +-
>> > 3 files changed, 5 insertions(+), 4 deletions(-)
>>
>> I've pushed these to linux-dlm next.
>
> Great, thanks! Should we send 1 and 3 to stable?
Yes please! :)
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread