* [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
@ 2018-11-07 0:21 Greg Kurz
2018-11-12 8:28 ` zhibin hu
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2018-11-07 0:21 UTC (permalink / raw)
To: qemu-devel; +Cc: Greg Kurz, Prasad J Pandit, zhibin hu
The assumption that the fid cannot be used by any other operation is
wrong. At least, nothing prevents a misbehaving client to create a
file with a given fid, and to pass this fid to some other operation
at the same time (ie, without waiting for the response to the creation
request). The call to v9fs_path_copy() performed by the worker thread
after the file was created can race with any access to the fid path
performed by some other thread. This causes use-after-free issues that
can be detected by ASAN with a custom 9p client.
Unlike other operations that only read the fid path, v9fs_co_open2()
does modify it. It should hence take the write lock.
Cc: P J P <ppandit@redhat.com>
Reported-by: zhibin hu <noirfate@gmail.com>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/cofile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 88791bc327ac..9c22837cda32 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp,
cred.fc_gid = gid;
/*
* Hold the directory fid lock so that directory path name
- * don't change. Read lock is fine because this fid cannot
- * be used by any other operation.
+ * don't change. Take the write lock to be sure this fid
+ * cannot be used by another operation.
*/
- v9fs_path_read_lock(s);
+ v9fs_path_write_lock(s);
v9fs_co_run_in_worker(
{
err = s->ops->open2(&s->ctx, &fidp->path,
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-07 0:21 [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2() Greg Kurz
@ 2018-11-12 8:28 ` zhibin hu
2018-11-12 10:00 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: zhibin hu @ 2018-11-12 8:28 UTC (permalink / raw)
To: groug; +Cc: qemu-devel, prasad
hi,
i use this patch with qemu 3.0.0 and it seems not fix completely.
[root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
-enable-kvm -net nic,model=e1000 -net
tap,helper=/usr/libexec/qemu-bridge-helper -hda
/var/lib/libvirt/images/test.qcow2 -fsdev
local,id=test_dev,path=/tmp,security_model=none -device
virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
functions and may produce false positives in some cases!
=================================================================
==5014==ERROR: AddressSanitizer: heap-use-after-free on address
0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp 0x7f9053444118
READ of size 2 at 0x60200014fc50 thread T17
#0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
#1 0x7f90c8452693 in g_path_get_basename
(/lib64/libglib-2.0.so.0+0x39693)
#2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
#3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
#4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
#5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
#6 0x559c1c145976 in coroutine_trampoline util/coroutine-ucontext.c:116
#7 0x7f90bd6855ff in __correctly_grouped_prefixwc
(/lib64/libc.so.6+0x4c5ff)
0x60200014fc50 is located 0 bytes inside of 2-byte region
[0x60200014fc50,0x60200014fc52)
freed by thread T0 here:
#0 0x7f90c90db880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
#2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
#3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
#4 0x559c1c145976 in coroutine_trampoline util/coroutine-ucontext.c:116
#5 0x7f90bd6855ff in __correctly_grouped_prefixwc
(/lib64/libc.so.6+0x4c5ff)
previously allocated by thread T4 here:
#0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
#1 0x7f90bd743c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
Thread T17 created by T16 here:
#0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
#1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
#2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
#3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
#4 0x559c1c1043e0 in qemu_thread_start util/qemu-thread-posix.c:504
#5 0x7f90bd9ff593 in start_thread (/lib64/libpthread.so.0+0x7593)
Thread T16 created by T0 here:
#0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
#1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
#2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
#3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
#4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
#5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
#6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
#7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
#8 0x7f90c84658ac in g_main_context_dispatch
(/lib64/libglib-2.0.so.0+0x4c8ac)
Thread T4 created by T0 here:
#0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
#1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
#2 0x559c1b2e7258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
#3 0x559c1b2e7a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
#4 0x559c1b583f0c in x86_cpu_realizefn
/root/qemu-3.0.0/target/i386/cpu.c:4996
#5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
#6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
#7 0x559c1bde2b0e in object_property_set qom/object.c:1176
#8 0x559c1bde9b00 in object_property_set_qobject qom/qom-qobject.c:27
#9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
#10 0x559c1b4f2452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
#11 0x559c1b4f2993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
#12 0x559c1b4fc9cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
#13 0x559c1b4fe81d in pc_init_v3_0
/root/qemu-3.0.0/hw/i386/pc_piix.c:438
#14 0x559c1b890f29 in machine_run_board_init hw/core/machine.c:830
#15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
#16 0x7f90bd65c11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
SUMMARY: AddressSanitizer: heap-use-after-free
(/lib64/libasan.so.5+0x9997c)
Shadow bytes around the buggy address:
0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
=>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==5014==ABORTING
thanks.
On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> The assumption that the fid cannot be used by any other operation is
> wrong. At least, nothing prevents a misbehaving client to create a
> file with a given fid, and to pass this fid to some other operation
> at the same time (ie, without waiting for the response to the creation
> request). The call to v9fs_path_copy() performed by the worker thread
> after the file was created can race with any access to the fid path
> performed by some other thread. This causes use-after-free issues that
> can be detected by ASAN with a custom 9p client.
>
> Unlike other operations that only read the fid path, v9fs_co_open2()
> does modify it. It should hence take the write lock.
>
> Cc: P J P <ppandit@redhat.com>
> Reported-by: zhibin hu <noirfate@gmail.com>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
> hw/9pfs/cofile.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> index 88791bc327ac..9c22837cda32 100644
> --- a/hw/9pfs/cofile.c
> +++ b/hw/9pfs/cofile.c
> @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu,
> V9fsFidState *fidp,
> cred.fc_gid = gid;
> /*
> * Hold the directory fid lock so that directory path name
> - * don't change. Read lock is fine because this fid cannot
> - * be used by any other operation.
> + * don't change. Take the write lock to be sure this fid
> + * cannot be used by another operation.
> */
> - v9fs_path_read_lock(s);
> + v9fs_path_write_lock(s);
> v9fs_co_run_in_worker(
> {
> err = s->ops->open2(&s->ctx, &fidp->path,
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-12 8:28 ` zhibin hu
@ 2018-11-12 10:00 ` Greg Kurz
2018-11-12 11:05 ` zhibin hu
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2018-11-12 10:00 UTC (permalink / raw)
To: zhibin hu; +Cc: qemu-devel, prasad
On Mon, 12 Nov 2018 16:28:28 +0800
zhibin hu <noirfate@gmail.com> wrote:
> hi,
>
> i use this patch with qemu 3.0.0 and it seems not fix completely.
>
> [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> -enable-kvm -net nic,model=e1000 -net
> tap,helper=/usr/libexec/qemu-bridge-helper -hda
> /var/lib/libvirt/images/test.qcow2 -fsdev
> local,id=test_dev,path=/tmp,security_model=none -device
> virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> functions and may produce false positives in some cases!
> =================================================================
> ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp 0x7f9053444118
> READ of size 2 at 0x60200014fc50 thread T17
> #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> #1 0x7f90c8452693 in g_path_get_basename
> (/lib64/libglib-2.0.so.0+0x39693)
> #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> #6 0x559c1c145976 in coroutine_trampoline util/coroutine-ucontext.c:116
> #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> (/lib64/libc.so.6+0x4c5ff)
>
> 0x60200014fc50 is located 0 bytes inside of 2-byte region
> [0x60200014fc50,0x60200014fc52)
> freed by thread T0 here:
> #0 0x7f90c90db880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
Ah, so we have a similar issue when creating files with the older
9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
Cheers,
--
Greg
> #4 0x559c1c145976 in coroutine_trampoline util/coroutine-ucontext.c:116
> #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> (/lib64/libc.so.6+0x4c5ff)
>
> previously allocated by thread T4 here:
> #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> #1 0x7f90bd743c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
>
> Thread T17 created by T16 here:
> #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> #4 0x559c1c1043e0 in qemu_thread_start util/qemu-thread-posix.c:504
> #5 0x7f90bd9ff593 in start_thread (/lib64/libpthread.so.0+0x7593)
>
> Thread T16 created by T0 here:
> #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> #8 0x7f90c84658ac in g_main_context_dispatch
> (/lib64/libglib-2.0.so.0+0x4c8ac)
>
> Thread T4 created by T0 here:
> #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> #2 0x559c1b2e7258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> #3 0x559c1b2e7a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> #4 0x559c1b583f0c in x86_cpu_realizefn
> /root/qemu-3.0.0/target/i386/cpu.c:4996
> #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> #8 0x559c1bde9b00 in object_property_set_qobject qom/qom-qobject.c:27
> #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> #10 0x559c1b4f2452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> #11 0x559c1b4f2993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> #12 0x559c1b4fc9cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> #13 0x559c1b4fe81d in pc_init_v3_0
> /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> #14 0x559c1b890f29 in machine_run_board_init hw/core/machine.c:830
> #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> #16 0x7f90bd65c11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
>
> SUMMARY: AddressSanitizer: heap-use-after-free
> (/lib64/libasan.so.5+0x9997c)
> Shadow bytes around the buggy address:
> 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==5014==ABORTING
>
> thanks.
>
> On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
>
> > The assumption that the fid cannot be used by any other operation is
> > wrong. At least, nothing prevents a misbehaving client to create a
> > file with a given fid, and to pass this fid to some other operation
> > at the same time (ie, without waiting for the response to the creation
> > request). The call to v9fs_path_copy() performed by the worker thread
> > after the file was created can race with any access to the fid path
> > performed by some other thread. This causes use-after-free issues that
> > can be detected by ASAN with a custom 9p client.
> >
> > Unlike other operations that only read the fid path, v9fs_co_open2()
> > does modify it. It should hence take the write lock.
> >
> > Cc: P J P <ppandit@redhat.com>
> > Reported-by: zhibin hu <noirfate@gmail.com>
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> > hw/9pfs/cofile.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > index 88791bc327ac..9c22837cda32 100644
> > --- a/hw/9pfs/cofile.c
> > +++ b/hw/9pfs/cofile.c
> > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu,
> > V9fsFidState *fidp,
> > cred.fc_gid = gid;
> > /*
> > * Hold the directory fid lock so that directory path name
> > - * don't change. Read lock is fine because this fid cannot
> > - * be used by any other operation.
> > + * don't change. Take the write lock to be sure this fid
> > + * cannot be used by another operation.
> > */
> > - v9fs_path_read_lock(s);
> > + v9fs_path_write_lock(s);
> > v9fs_co_run_in_worker(
> > {
> > err = s->ops->open2(&s->ctx, &fidp->path,
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-12 10:00 ` Greg Kurz
@ 2018-11-12 11:05 ` zhibin hu
2018-11-12 11:19 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: zhibin hu @ 2018-11-12 11:05 UTC (permalink / raw)
To: groug; +Cc: qemu-devel, prasad
yes, and this :
==6094==ERROR: AddressSanitizer: heap-use-after-free on address
0x6020000e6751 at pc 0x562a8dc492b8 bp 0x7f6805d2fa10 sp 0x7f6805d2fa00
READ of size 1 at 0x6020000e6751 thread T21
#0 0x562a8dc492b7 in local_open_nofollow hw/9pfs/9p-local.c:59
#1 0x562a8dc49361 in local_opendir_nofollow hw/9pfs/9p-local.c:92
#2 0x562a8dc4bd6e in local_mknod hw/9pfs/9p-local.c:662
#3 0x562a8dc521de in v9fs_co_mknod hw/9pfs/cofs.c:200
#4 0x562a8dc4413e in v9fs_mknod hw/9pfs/9p.c:3044
#5 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
#6 0x7f68713635ff in __correctly_grouped_prefixwc
(/lib64/libc.so.6+0x4c5ff)
0x6020000e6751 is located 1 bytes inside of 2-byte region
[0x6020000e6750,0x6020000e6752)
freed by thread T0 here:
#0 0x7f687cdb9880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7f687c1494d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
#2 0x562a8dc30ce4 in v9fs_path_copy hw/9pfs/9p.c:195
#3 0x562a8dc3f0f3 in v9fs_create hw/9pfs/9p.c:2286
#4 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
#5 0x7f68713635ff in __correctly_grouped_prefixwc
(/lib64/libc.so.6+0x4c5ff)
previously allocated by thread T5 here:
#0 0x7f687cdb9c48 in malloc (/lib64/libasan.so.5+0xeec48)
#1 0x7f6871421c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
Thread T21 created by T0 here:
#0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
#1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
#2 0x562a8e5adbe4 in do_spawn_thread util/thread-pool.c:135
#3 0x562a8e5adc73 in spawn_thread_bh_fn util/thread-pool.c:143
#4 0x562a8e5aa4d0 in aio_bh_call util/async.c:90
#5 0x562a8e5aa787 in aio_bh_poll util/async.c:118
#6 0x562a8e5b65bd in aio_dispatch util/aio-posix.c:436
#7 0x562a8e5ab26f in aio_ctx_dispatch util/async.c:261
#8 0x7f687c1438ac in g_main_context_dispatch
(/lib64/libglib-2.0.so.0+0x4c8ac)
Thread T5 created by T0 here:
#0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
#1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
#2 0x562a8d7a2258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
#3 0x562a8d7a2a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
#4 0x562a8da3ef0c in x86_cpu_realizefn
/root/qemu-3.0.0/target/i386/cpu.c:4996
#5 0x562a8dd3a1e8 in device_set_realized hw/core/qdev.c:826
#6 0x562a8e2a1e62 in property_set_bool qom/object.c:1984
#7 0x562a8e29db0e in object_property_set qom/object.c:1176
#8 0x562a8e2a4b00 in object_property_set_qobject qom/qom-qobject.c:27
#9 0x562a8e29de2b in object_property_set_bool qom/object.c:1242
#10 0x562a8d9ad452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
#11 0x562a8d9ad993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
#12 0x562a8d9b79cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
#13 0x562a8d9b981d in pc_init_v3_0
/root/qemu-3.0.0/hw/i386/pc_piix.c:438
#14 0x562a8dd4bf29 in machine_run_board_init hw/core/machine.c:830
#15 0x562a8db8d46e in main /root/qemu-3.0.0/vl.c:4516
#16 0x7f687133a11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
SUMMARY: AddressSanitizer: heap-use-after-free hw/9pfs/9p-local.c:59 in
local_open_nofollow
Shadow bytes around the buggy address:
0x0c0480014c90: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0480014ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0480014cb0: fa fa fa fa fa fa fd fd fa fa fd fa fa fa fd fa
0x0c0480014cc0: fa fa fa fa fa fa fa fa fa fa fd fd fa fa fd fa
0x0c0480014cd0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
=>0x0c0480014ce0: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fa fa
0x0c0480014cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
0x0c0480014d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x0c0480014d10: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
0x0c0480014d20: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fd fa
0x0c0480014d30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==6094==ABORTING
thanks.
On Mon, Nov 12, 2018 at 6:00 PM Greg Kurz <groug@kaod.org> wrote:
> On Mon, 12 Nov 2018 16:28:28 +0800
> zhibin hu <noirfate@gmail.com> wrote:
>
> > hi,
> >
> > i use this patch with qemu 3.0.0 and it seems not fix completely.
> >
> > [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> > -enable-kvm -net nic,model=e1000 -net
> > tap,helper=/usr/libexec/qemu-bridge-helper -hda
> > /var/lib/libvirt/images/test.qcow2 -fsdev
> > local,id=test_dev,path=/tmp,security_model=none -device
> > virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> > ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> > functions and may produce false positives in some cases!
> > =================================================================
> > ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> > 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp 0x7f9053444118
> > READ of size 2 at 0x60200014fc50 thread T17
> > #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> > #1 0x7f90c8452693 in g_path_get_basename
> > (/lib64/libglib-2.0.so.0+0x39693)
> > #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> > #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> > #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> > #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> > #6 0x559c1c145976 in coroutine_trampoline
> util/coroutine-ucontext.c:116
> > #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > (/lib64/libc.so.6+0x4c5ff)
> >
> > 0x60200014fc50 is located 0 bytes inside of 2-byte region
> > [0x60200014fc50,0x60200014fc52)
> > freed by thread T0 here:
> > #0 0x7f90c90db880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> > #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
>
> Ah, so we have a similar issue when creating files with the older
> 9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
>
> Cheers,
>
> --
> Greg
>
> > #4 0x559c1c145976 in coroutine_trampoline
> util/coroutine-ucontext.c:116
> > #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > (/lib64/libc.so.6+0x4c5ff)
> >
> > previously allocated by thread T4 here:
> > #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> > #1 0x7f90bd743c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
> >
> > Thread T17 created by T16 here:
> > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> > #4 0x559c1c1043e0 in qemu_thread_start util/qemu-thread-posix.c:504
> > #5 0x7f90bd9ff593 in start_thread (/lib64/libpthread.so.0+0x7593)
> >
> > Thread T16 created by T0 here:
> > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> > #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> > #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> > #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> > #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> > #8 0x7f90c84658ac in g_main_context_dispatch
> > (/lib64/libglib-2.0.so.0+0x4c8ac)
> >
> > Thread T4 created by T0 here:
> > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > #2 0x559c1b2e7258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> > #3 0x559c1b2e7a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > #4 0x559c1b583f0c in x86_cpu_realizefn
> > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> > #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> > #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> > #8 0x559c1bde9b00 in object_property_set_qobject qom/qom-qobject.c:27
> > #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> > #10 0x559c1b4f2452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > #11 0x559c1b4f2993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> > #12 0x559c1b4fc9cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > #13 0x559c1b4fe81d in pc_init_v3_0
> > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > #14 0x559c1b890f29 in machine_run_board_init hw/core/machine.c:830
> > #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> > #16 0x7f90bd65c11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> >
> > SUMMARY: AddressSanitizer: heap-use-after-free
> > (/lib64/libasan.so.5+0x9997c)
> > Shadow bytes around the buggy address:
> > 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> > 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> > =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> > 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> > 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> > 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> > 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> > 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> > Shadow byte legend (one shadow byte represents 8 application bytes):
> > Addressable: 00
> > Partially addressable: 01 02 03 04 05 06 07
> > Heap left redzone: fa
> > Freed heap region: fd
> > Stack left redzone: f1
> > Stack mid redzone: f2
> > Stack right redzone: f3
> > Stack after return: f5
> > Stack use after scope: f8
> > Global redzone: f9
> > Global init order: f6
> > Poisoned by user: f7
> > Container overflow: fc
> > Array cookie: ac
> > Intra object redzone: bb
> > ASan internal: fe
> > Left alloca redzone: ca
> > Right alloca redzone: cb
> > ==5014==ABORTING
> >
> > thanks.
> >
> > On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> >
> > > The assumption that the fid cannot be used by any other operation is
> > > wrong. At least, nothing prevents a misbehaving client to create a
> > > file with a given fid, and to pass this fid to some other operation
> > > at the same time (ie, without waiting for the response to the creation
> > > request). The call to v9fs_path_copy() performed by the worker thread
> > > after the file was created can race with any access to the fid path
> > > performed by some other thread. This causes use-after-free issues that
> > > can be detected by ASAN with a custom 9p client.
> > >
> > > Unlike other operations that only read the fid path, v9fs_co_open2()
> > > does modify it. It should hence take the write lock.
> > >
> > > Cc: P J P <ppandit@redhat.com>
> > > Reported-by: zhibin hu <noirfate@gmail.com>
> > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > ---
> > > hw/9pfs/cofile.c | 6 +++---
> > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > > index 88791bc327ac..9c22837cda32 100644
> > > --- a/hw/9pfs/cofile.c
> > > +++ b/hw/9pfs/cofile.c
> > > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu,
> > > V9fsFidState *fidp,
> > > cred.fc_gid = gid;
> > > /*
> > > * Hold the directory fid lock so that directory path name
> > > - * don't change. Read lock is fine because this fid cannot
> > > - * be used by any other operation.
> > > + * don't change. Take the write lock to be sure this fid
> > > + * cannot be used by another operation.
> > > */
> > > - v9fs_path_read_lock(s);
> > > + v9fs_path_write_lock(s);
> > > v9fs_co_run_in_worker(
> > > {
> > > err = s->ops->open2(&s->ctx, &fidp->path,
> > >
> > >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-12 11:05 ` zhibin hu
@ 2018-11-12 11:19 ` Greg Kurz
2018-11-12 14:39 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2018-11-12 11:19 UTC (permalink / raw)
To: zhibin hu; +Cc: qemu-devel, prasad
On Mon, 12 Nov 2018 19:05:59 +0800
zhibin hu <noirfate@gmail.com> wrote:
> yes, and this :
>
Yeah, all call sites of v9fs_path_copy() in v9fs_create() are called in the
context of the main thread. They may race with any other access to the fid
path performed by some other command in the context of a worker thread. My
first guess is that v9fs_create() should take the write lock before writing
to the fid path.
BTW, if you could share all the reproducers you already have for these
heap-use-after-free issues, it would be appreciated, and probably speed
up the fixing.
> ==6094==ERROR: AddressSanitizer: heap-use-after-free on address
> 0x6020000e6751 at pc 0x562a8dc492b8 bp 0x7f6805d2fa10 sp 0x7f6805d2fa00
> READ of size 1 at 0x6020000e6751 thread T21
> #0 0x562a8dc492b7 in local_open_nofollow hw/9pfs/9p-local.c:59
> #1 0x562a8dc49361 in local_opendir_nofollow hw/9pfs/9p-local.c:92
> #2 0x562a8dc4bd6e in local_mknod hw/9pfs/9p-local.c:662
> #3 0x562a8dc521de in v9fs_co_mknod hw/9pfs/cofs.c:200
> #4 0x562a8dc4413e in v9fs_mknod hw/9pfs/9p.c:3044
> #5 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
> #6 0x7f68713635ff in __correctly_grouped_prefixwc
> (/lib64/libc.so.6+0x4c5ff)
>
> 0x6020000e6751 is located 1 bytes inside of 2-byte region
> [0x6020000e6750,0x6020000e6752)
> freed by thread T0 here:
> #0 0x7f687cdb9880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> #1 0x7f687c1494d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> #2 0x562a8dc30ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> #3 0x562a8dc3f0f3 in v9fs_create hw/9pfs/9p.c:2286
> #4 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
> #5 0x7f68713635ff in __correctly_grouped_prefixwc
> (/lib64/libc.so.6+0x4c5ff)
>
> previously allocated by thread T5 here:
> #0 0x7f687cdb9c48 in malloc (/lib64/libasan.so.5+0xeec48)
> #1 0x7f6871421c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
>
> Thread T21 created by T0 here:
> #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> #1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
> #2 0x562a8e5adbe4 in do_spawn_thread util/thread-pool.c:135
> #3 0x562a8e5adc73 in spawn_thread_bh_fn util/thread-pool.c:143
> #4 0x562a8e5aa4d0 in aio_bh_call util/async.c:90
> #5 0x562a8e5aa787 in aio_bh_poll util/async.c:118
> #6 0x562a8e5b65bd in aio_dispatch util/aio-posix.c:436
> #7 0x562a8e5ab26f in aio_ctx_dispatch util/async.c:261
> #8 0x7f687c1438ac in g_main_context_dispatch
> (/lib64/libglib-2.0.so.0+0x4c8ac)
>
> Thread T5 created by T0 here:
> #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> #1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
> #2 0x562a8d7a2258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> #3 0x562a8d7a2a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> #4 0x562a8da3ef0c in x86_cpu_realizefn
> /root/qemu-3.0.0/target/i386/cpu.c:4996
> #5 0x562a8dd3a1e8 in device_set_realized hw/core/qdev.c:826
> #6 0x562a8e2a1e62 in property_set_bool qom/object.c:1984
> #7 0x562a8e29db0e in object_property_set qom/object.c:1176
> #8 0x562a8e2a4b00 in object_property_set_qobject qom/qom-qobject.c:27
> #9 0x562a8e29de2b in object_property_set_bool qom/object.c:1242
> #10 0x562a8d9ad452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> #11 0x562a8d9ad993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> #12 0x562a8d9b79cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> #13 0x562a8d9b981d in pc_init_v3_0
> /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> #14 0x562a8dd4bf29 in machine_run_board_init hw/core/machine.c:830
> #15 0x562a8db8d46e in main /root/qemu-3.0.0/vl.c:4516
> #16 0x7f687133a11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
>
> SUMMARY: AddressSanitizer: heap-use-after-free hw/9pfs/9p-local.c:59 in
> local_open_nofollow
> Shadow bytes around the buggy address:
> 0x0c0480014c90: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c0480014ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c0480014cb0: fa fa fa fa fa fa fd fd fa fa fd fa fa fa fd fa
> 0x0c0480014cc0: fa fa fa fa fa fa fa fa fa fa fd fd fa fa fd fa
> 0x0c0480014cd0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> =>0x0c0480014ce0: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fa fa
> 0x0c0480014cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
> 0x0c0480014d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> 0x0c0480014d10: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> 0x0c0480014d20: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fd fa
> 0x0c0480014d30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==6094==ABORTING
>
> thanks.
>
> On Mon, Nov 12, 2018 at 6:00 PM Greg Kurz <groug@kaod.org> wrote:
>
> > On Mon, 12 Nov 2018 16:28:28 +0800
> > zhibin hu <noirfate@gmail.com> wrote:
> >
> > > hi,
> > >
> > > i use this patch with qemu 3.0.0 and it seems not fix completely.
> > >
> > > [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> > > -enable-kvm -net nic,model=e1000 -net
> > > tap,helper=/usr/libexec/qemu-bridge-helper -hda
> > > /var/lib/libvirt/images/test.qcow2 -fsdev
> > > local,id=test_dev,path=/tmp,security_model=none -device
> > > virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> > > ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> > > functions and may produce false positives in some cases!
> > > =================================================================
> > > ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> > > 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp 0x7f9053444118
> > > READ of size 2 at 0x60200014fc50 thread T17
> > > #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> > > #1 0x7f90c8452693 in g_path_get_basename
> > > (/lib64/libglib-2.0.so.0+0x39693)
> > > #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> > > #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> > > #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> > > #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> > > #6 0x559c1c145976 in coroutine_trampoline
> > util/coroutine-ucontext.c:116
> > > #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > (/lib64/libc.so.6+0x4c5ff)
> > >
> > > 0x60200014fc50 is located 0 bytes inside of 2-byte region
> > > [0x60200014fc50,0x60200014fc52)
> > > freed by thread T0 here:
> > > #0 0x7f90c90db880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> > > #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
> >
> > Ah, so we have a similar issue when creating files with the older
> > 9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
> >
> > Cheers,
> >
> > --
> > Greg
> >
> > > #4 0x559c1c145976 in coroutine_trampoline
> > util/coroutine-ucontext.c:116
> > > #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > (/lib64/libc.so.6+0x4c5ff)
> > >
> > > previously allocated by thread T4 here:
> > > #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > #1 0x7f90bd743c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
> > >
> > > Thread T17 created by T16 here:
> > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> > > #4 0x559c1c1043e0 in qemu_thread_start util/qemu-thread-posix.c:504
> > > #5 0x7f90bd9ff593 in start_thread (/lib64/libpthread.so.0+0x7593)
> > >
> > > Thread T16 created by T0 here:
> > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> > > #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> > > #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> > > #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> > > #8 0x7f90c84658ac in g_main_context_dispatch
> > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > >
> > > Thread T4 created by T0 here:
> > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > #2 0x559c1b2e7258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> > > #3 0x559c1b2e7a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > > #4 0x559c1b583f0c in x86_cpu_realizefn
> > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> > > #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> > > #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> > > #8 0x559c1bde9b00 in object_property_set_qobject qom/qom-qobject.c:27
> > > #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> > > #10 0x559c1b4f2452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > #11 0x559c1b4f2993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > #12 0x559c1b4fc9cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > #13 0x559c1b4fe81d in pc_init_v3_0
> > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > #14 0x559c1b890f29 in machine_run_board_init hw/core/machine.c:830
> > > #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> > > #16 0x7f90bd65c11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> > >
> > > SUMMARY: AddressSanitizer: heap-use-after-free
> > > (/lib64/libasan.so.5+0x9997c)
> > > Shadow bytes around the buggy address:
> > > 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> > > 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> > > 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> > > 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> > > 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > Shadow byte legend (one shadow byte represents 8 application bytes):
> > > Addressable: 00
> > > Partially addressable: 01 02 03 04 05 06 07
> > > Heap left redzone: fa
> > > Freed heap region: fd
> > > Stack left redzone: f1
> > > Stack mid redzone: f2
> > > Stack right redzone: f3
> > > Stack after return: f5
> > > Stack use after scope: f8
> > > Global redzone: f9
> > > Global init order: f6
> > > Poisoned by user: f7
> > > Container overflow: fc
> > > Array cookie: ac
> > > Intra object redzone: bb
> > > ASan internal: fe
> > > Left alloca redzone: ca
> > > Right alloca redzone: cb
> > > ==5014==ABORTING
> > >
> > > thanks.
> > >
> > > On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> > >
> > > > The assumption that the fid cannot be used by any other operation is
> > > > wrong. At least, nothing prevents a misbehaving client to create a
> > > > file with a given fid, and to pass this fid to some other operation
> > > > at the same time (ie, without waiting for the response to the creation
> > > > request). The call to v9fs_path_copy() performed by the worker thread
> > > > after the file was created can race with any access to the fid path
> > > > performed by some other thread. This causes use-after-free issues that
> > > > can be detected by ASAN with a custom 9p client.
> > > >
> > > > Unlike other operations that only read the fid path, v9fs_co_open2()
> > > > does modify it. It should hence take the write lock.
> > > >
> > > > Cc: P J P <ppandit@redhat.com>
> > > > Reported-by: zhibin hu <noirfate@gmail.com>
> > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > > ---
> > > > hw/9pfs/cofile.c | 6 +++---
> > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > > > index 88791bc327ac..9c22837cda32 100644
> > > > --- a/hw/9pfs/cofile.c
> > > > +++ b/hw/9pfs/cofile.c
> > > > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu,
> > > > V9fsFidState *fidp,
> > > > cred.fc_gid = gid;
> > > > /*
> > > > * Hold the directory fid lock so that directory path name
> > > > - * don't change. Read lock is fine because this fid cannot
> > > > - * be used by any other operation.
> > > > + * don't change. Take the write lock to be sure this fid
> > > > + * cannot be used by another operation.
> > > > */
> > > > - v9fs_path_read_lock(s);
> > > > + v9fs_path_write_lock(s);
> > > > v9fs_co_run_in_worker(
> > > > {
> > > > err = s->ops->open2(&s->ctx, &fidp->path,
> > > >
> > > >
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-12 11:19 ` Greg Kurz
@ 2018-11-12 14:39 ` Greg Kurz
2018-11-13 3:35 ` zhibin hu
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kurz @ 2018-11-12 14:39 UTC (permalink / raw)
To: zhibin hu; +Cc: qemu-devel, prasad
On Mon, 12 Nov 2018 12:19:29 +0100
Greg Kurz <groug@kaod.org> wrote:
> On Mon, 12 Nov 2018 19:05:59 +0800
> zhibin hu <noirfate@gmail.com> wrote:
>
> > yes, and this :
> >
>
> Yeah, all call sites of v9fs_path_copy() in v9fs_create() are called in the
> context of the main thread. They may race with any other access to the fid
> path performed by some other command in the context of a worker thread. My
> first guess is that v9fs_create() should take the write lock before writing
> to the fid path.
>
I think this call to v9fs_path_copy() in v9fs_walk() can also race:
if (fid == newfid) {
if (fidp->fid_type != P9_FID_NONE) {
err = -EINVAL;
goto out;
}
v9fs_path_copy(&fidp->path, &path);
} else {
Worse, since v9fs_co_open2() may overwrite the fid path from a worker
thread, it seems that some more code might require to run with the read
lock taken...
> BTW, if you could share all the reproducers you already have for these
> heap-use-after-free issues, it would be appreciated, and probably speed
> up the fixing.
>
> > ==6094==ERROR: AddressSanitizer: heap-use-after-free on address
> > 0x6020000e6751 at pc 0x562a8dc492b8 bp 0x7f6805d2fa10 sp 0x7f6805d2fa00
> > READ of size 1 at 0x6020000e6751 thread T21
> > #0 0x562a8dc492b7 in local_open_nofollow hw/9pfs/9p-local.c:59
> > #1 0x562a8dc49361 in local_opendir_nofollow hw/9pfs/9p-local.c:92
> > #2 0x562a8dc4bd6e in local_mknod hw/9pfs/9p-local.c:662
> > #3 0x562a8dc521de in v9fs_co_mknod hw/9pfs/cofs.c:200
> > #4 0x562a8dc4413e in v9fs_mknod hw/9pfs/9p.c:3044
> > #5 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
> > #6 0x7f68713635ff in __correctly_grouped_prefixwc
> > (/lib64/libc.so.6+0x4c5ff)
> >
> > 0x6020000e6751 is located 1 bytes inside of 2-byte region
> > [0x6020000e6750,0x6020000e6752)
> > freed by thread T0 here:
> > #0 0x7f687cdb9880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> > #1 0x7f687c1494d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > #2 0x562a8dc30ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > #3 0x562a8dc3f0f3 in v9fs_create hw/9pfs/9p.c:2286
> > #4 0x562a8e600976 in coroutine_trampoline util/coroutine-ucontext.c:116
> > #5 0x7f68713635ff in __correctly_grouped_prefixwc
> > (/lib64/libc.so.6+0x4c5ff)
> >
> > previously allocated by thread T5 here:
> > #0 0x7f687cdb9c48 in malloc (/lib64/libasan.so.5+0xeec48)
> > #1 0x7f6871421c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
> >
> > Thread T21 created by T0 here:
> > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > #1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
> > #2 0x562a8e5adbe4 in do_spawn_thread util/thread-pool.c:135
> > #3 0x562a8e5adc73 in spawn_thread_bh_fn util/thread-pool.c:143
> > #4 0x562a8e5aa4d0 in aio_bh_call util/async.c:90
> > #5 0x562a8e5aa787 in aio_bh_poll util/async.c:118
> > #6 0x562a8e5b65bd in aio_dispatch util/aio-posix.c:436
> > #7 0x562a8e5ab26f in aio_ctx_dispatch util/async.c:261
> > #8 0x7f687c1438ac in g_main_context_dispatch
> > (/lib64/libglib-2.0.so.0+0x4c8ac)
> >
> > Thread T5 created by T0 here:
> > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > #1 0x562a8e5bf61e in qemu_thread_create util/qemu-thread-posix.c:534
> > #2 0x562a8d7a2258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> > #3 0x562a8d7a2a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > #4 0x562a8da3ef0c in x86_cpu_realizefn
> > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > #5 0x562a8dd3a1e8 in device_set_realized hw/core/qdev.c:826
> > #6 0x562a8e2a1e62 in property_set_bool qom/object.c:1984
> > #7 0x562a8e29db0e in object_property_set qom/object.c:1176
> > #8 0x562a8e2a4b00 in object_property_set_qobject qom/qom-qobject.c:27
> > #9 0x562a8e29de2b in object_property_set_bool qom/object.c:1242
> > #10 0x562a8d9ad452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > #11 0x562a8d9ad993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> > #12 0x562a8d9b79cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > #13 0x562a8d9b981d in pc_init_v3_0
> > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > #14 0x562a8dd4bf29 in machine_run_board_init hw/core/machine.c:830
> > #15 0x562a8db8d46e in main /root/qemu-3.0.0/vl.c:4516
> > #16 0x7f687133a11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> >
> > SUMMARY: AddressSanitizer: heap-use-after-free hw/9pfs/9p-local.c:59 in
> > local_open_nofollow
> > Shadow bytes around the buggy address:
> > 0x0c0480014c90: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa
> > 0x0c0480014ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > 0x0c0480014cb0: fa fa fa fa fa fa fd fd fa fa fd fa fa fa fd fa
> > 0x0c0480014cc0: fa fa fa fa fa fa fa fa fa fa fd fd fa fa fd fa
> > 0x0c0480014cd0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > =>0x0c0480014ce0: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fa fa
> > 0x0c0480014cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
> > 0x0c0480014d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > 0x0c0480014d10: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > 0x0c0480014d20: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fd fa
> > 0x0c0480014d30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
> > Shadow byte legend (one shadow byte represents 8 application bytes):
> > Addressable: 00
> > Partially addressable: 01 02 03 04 05 06 07
> > Heap left redzone: fa
> > Freed heap region: fd
> > Stack left redzone: f1
> > Stack mid redzone: f2
> > Stack right redzone: f3
> > Stack after return: f5
> > Stack use after scope: f8
> > Global redzone: f9
> > Global init order: f6
> > Poisoned by user: f7
> > Container overflow: fc
> > Array cookie: ac
> > Intra object redzone: bb
> > ASan internal: fe
> > Left alloca redzone: ca
> > Right alloca redzone: cb
> > ==6094==ABORTING
> >
> > thanks.
> >
> > On Mon, Nov 12, 2018 at 6:00 PM Greg Kurz <groug@kaod.org> wrote:
> >
> > > On Mon, 12 Nov 2018 16:28:28 +0800
> > > zhibin hu <noirfate@gmail.com> wrote:
> > >
> > > > hi,
> > > >
> > > > i use this patch with qemu 3.0.0 and it seems not fix completely.
> > > >
> > > > [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> > > > -enable-kvm -net nic,model=e1000 -net
> > > > tap,helper=/usr/libexec/qemu-bridge-helper -hda
> > > > /var/lib/libvirt/images/test.qcow2 -fsdev
> > > > local,id=test_dev,path=/tmp,security_model=none -device
> > > > virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> > > > ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> > > > functions and may produce false positives in some cases!
> > > > =================================================================
> > > > ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> > > > 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp 0x7f9053444118
> > > > READ of size 2 at 0x60200014fc50 thread T17
> > > > #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> > > > #1 0x7f90c8452693 in g_path_get_basename
> > > > (/lib64/libglib-2.0.so.0+0x39693)
> > > > #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> > > > #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> > > > #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> > > > #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> > > > #6 0x559c1c145976 in coroutine_trampoline
> > > util/coroutine-ucontext.c:116
> > > > #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > (/lib64/libc.so.6+0x4c5ff)
> > > >
> > > > 0x60200014fc50 is located 0 bytes inside of 2-byte region
> > > > [0x60200014fc50,0x60200014fc52)
> > > > freed by thread T0 here:
> > > > #0 0x7f90c90db880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
> > > > #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > > #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > > #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
> > >
> > > Ah, so we have a similar issue when creating files with the older
> > > 9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
> > >
> > > Cheers,
> > >
> > > --
> > > Greg
> > >
> > > > #4 0x559c1c145976 in coroutine_trampoline
> > > util/coroutine-ucontext.c:116
> > > > #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > (/lib64/libc.so.6+0x4c5ff)
> > > >
> > > > previously allocated by thread T4 here:
> > > > #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > > #1 0x7f90bd743c37 in __GI___vasprintf_chk (/lib64/libc.so.6+0x10ac37)
> > > >
> > > > Thread T17 created by T16 here:
> > > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> > > > #4 0x559c1c1043e0 in qemu_thread_start util/qemu-thread-posix.c:504
> > > > #5 0x7f90bd9ff593 in start_thread (/lib64/libpthread.so.0+0x7593)
> > > >
> > > > Thread T16 created by T0 here:
> > > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > > #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> > > > #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> > > > #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> > > > #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> > > > #8 0x7f90c84658ac in g_main_context_dispatch
> > > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > > >
> > > > Thread T4 created by T0 here:
> > > > #0 0x7f90c9038443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > > #1 0x559c1c10461e in qemu_thread_create util/qemu-thread-posix.c:534
> > > > #2 0x559c1b2e7258 in qemu_kvm_start_vcpu /root/qemu-3.0.0/cpus.c:1935
> > > > #3 0x559c1b2e7a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > > > #4 0x559c1b583f0c in x86_cpu_realizefn
> > > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > > #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> > > > #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> > > > #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> > > > #8 0x559c1bde9b00 in object_property_set_qobject qom/qom-qobject.c:27
> > > > #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> > > > #10 0x559c1b4f2452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > > #11 0x559c1b4f2993 in pc_cpus_init /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > > #12 0x559c1b4fc9cb in pc_init1 /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > > #13 0x559c1b4fe81d in pc_init_v3_0
> > > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > > #14 0x559c1b890f29 in machine_run_board_init hw/core/machine.c:830
> > > > #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> > > > #16 0x7f90bd65c11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> > > >
> > > > SUMMARY: AddressSanitizer: heap-use-after-free
> > > > (/lib64/libasan.so.5+0x9997c)
> > > > Shadow bytes around the buggy address:
> > > > 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > > 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > > 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> > > > 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> > > > 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> > > > 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> > > > 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > > 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > > Shadow byte legend (one shadow byte represents 8 application bytes):
> > > > Addressable: 00
> > > > Partially addressable: 01 02 03 04 05 06 07
> > > > Heap left redzone: fa
> > > > Freed heap region: fd
> > > > Stack left redzone: f1
> > > > Stack mid redzone: f2
> > > > Stack right redzone: f3
> > > > Stack after return: f5
> > > > Stack use after scope: f8
> > > > Global redzone: f9
> > > > Global init order: f6
> > > > Poisoned by user: f7
> > > > Container overflow: fc
> > > > Array cookie: ac
> > > > Intra object redzone: bb
> > > > ASan internal: fe
> > > > Left alloca redzone: ca
> > > > Right alloca redzone: cb
> > > > ==5014==ABORTING
> > > >
> > > > thanks.
> > > >
> > > > On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> > > >
> > > > > The assumption that the fid cannot be used by any other operation is
> > > > > wrong. At least, nothing prevents a misbehaving client to create a
> > > > > file with a given fid, and to pass this fid to some other operation
> > > > > at the same time (ie, without waiting for the response to the creation
> > > > > request). The call to v9fs_path_copy() performed by the worker thread
> > > > > after the file was created can race with any access to the fid path
> > > > > performed by some other thread. This causes use-after-free issues that
> > > > > can be detected by ASAN with a custom 9p client.
> > > > >
> > > > > Unlike other operations that only read the fid path, v9fs_co_open2()
> > > > > does modify it. It should hence take the write lock.
> > > > >
> > > > > Cc: P J P <ppandit@redhat.com>
> > > > > Reported-by: zhibin hu <noirfate@gmail.com>
> > > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > > > ---
> > > > > hw/9pfs/cofile.c | 6 +++---
> > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > > > > index 88791bc327ac..9c22837cda32 100644
> > > > > --- a/hw/9pfs/cofile.c
> > > > > +++ b/hw/9pfs/cofile.c
> > > > > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu,
> > > > > V9fsFidState *fidp,
> > > > > cred.fc_gid = gid;
> > > > > /*
> > > > > * Hold the directory fid lock so that directory path name
> > > > > - * don't change. Read lock is fine because this fid cannot
> > > > > - * be used by any other operation.
> > > > > + * don't change. Take the write lock to be sure this fid
> > > > > + * cannot be used by another operation.
> > > > > */
> > > > > - v9fs_path_read_lock(s);
> > > > > + v9fs_path_write_lock(s);
> > > > > v9fs_co_run_in_worker(
> > > > > {
> > > > > err = s->ops->open2(&s->ctx, &fidp->path,
> > > > >
> > > > >
> > >
> > >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-12 14:39 ` Greg Kurz
@ 2018-11-13 3:35 ` zhibin hu
2018-11-13 8:03 ` Greg Kurz
0 siblings, 1 reply; 8+ messages in thread
From: zhibin hu @ 2018-11-13 3:35 UTC (permalink / raw)
To: groug; +Cc: qemu-devel, prasad
Sorry, i have no time to make poc recently.
IMHO, the implementation of v9fs_path_copy is not secure, it first free the
original value and than copy the new value, there is a race.
So each caller must ensure the synchronization, maybe more locks are needed.
thanks.
On Mon, Nov 12, 2018 at 10:39 PM Greg Kurz <groug@kaod.org> wrote:
> On Mon, 12 Nov 2018 12:19:29 +0100
> Greg Kurz <groug@kaod.org> wrote:
>
> > On Mon, 12 Nov 2018 19:05:59 +0800
> > zhibin hu <noirfate@gmail.com> wrote:
> >
> > > yes, and this :
> > >
> >
> > Yeah, all call sites of v9fs_path_copy() in v9fs_create() are called in
> the
> > context of the main thread. They may race with any other access to the
> fid
> > path performed by some other command in the context of a worker thread.
> My
> > first guess is that v9fs_create() should take the write lock before
> writing
> > to the fid path.
> >
>
> I think this call to v9fs_path_copy() in v9fs_walk() can also race:
>
> if (fid == newfid) {
> if (fidp->fid_type != P9_FID_NONE) {
> err = -EINVAL;
> goto out;
> }
> v9fs_path_copy(&fidp->path, &path);
> } else {
>
>
> Worse, since v9fs_co_open2() may overwrite the fid path from a worker
> thread, it seems that some more code might require to run with the read
> lock taken...
>
> > BTW, if you could share all the reproducers you already have for these
> > heap-use-after-free issues, it would be appreciated, and probably speed
> > up the fixing.
> >
> > > ==6094==ERROR: AddressSanitizer: heap-use-after-free on address
> > > 0x6020000e6751 at pc 0x562a8dc492b8 bp 0x7f6805d2fa10 sp 0x7f6805d2fa00
> > > READ of size 1 at 0x6020000e6751 thread T21
> > > #0 0x562a8dc492b7 in local_open_nofollow hw/9pfs/9p-local.c:59
> > > #1 0x562a8dc49361 in local_opendir_nofollow hw/9pfs/9p-local.c:92
> > > #2 0x562a8dc4bd6e in local_mknod hw/9pfs/9p-local.c:662
> > > #3 0x562a8dc521de in v9fs_co_mknod hw/9pfs/cofs.c:200
> > > #4 0x562a8dc4413e in v9fs_mknod hw/9pfs/9p.c:3044
> > > #5 0x562a8e600976 in coroutine_trampoline
> util/coroutine-ucontext.c:116
> > > #6 0x7f68713635ff in __correctly_grouped_prefixwc
> > > (/lib64/libc.so.6+0x4c5ff)
> > >
> > > 0x6020000e6751 is located 1 bytes inside of 2-byte region
> > > [0x6020000e6750,0x6020000e6752)
> > > freed by thread T0 here:
> > > #0 0x7f687cdb9880 in __interceptor_free
> (/lib64/libasan.so.5+0xee880)
> > > #1 0x7f687c1494d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > #2 0x562a8dc30ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > #3 0x562a8dc3f0f3 in v9fs_create hw/9pfs/9p.c:2286
> > > #4 0x562a8e600976 in coroutine_trampoline
> util/coroutine-ucontext.c:116
> > > #5 0x7f68713635ff in __correctly_grouped_prefixwc
> > > (/lib64/libc.so.6+0x4c5ff)
> > >
> > > previously allocated by thread T5 here:
> > > #0 0x7f687cdb9c48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > #1 0x7f6871421c37 in __GI___vasprintf_chk
> (/lib64/libc.so.6+0x10ac37)
> > >
> > > Thread T21 created by T0 here:
> > > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > #1 0x562a8e5bf61e in qemu_thread_create
> util/qemu-thread-posix.c:534
> > > #2 0x562a8e5adbe4 in do_spawn_thread util/thread-pool.c:135
> > > #3 0x562a8e5adc73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > #4 0x562a8e5aa4d0 in aio_bh_call util/async.c:90
> > > #5 0x562a8e5aa787 in aio_bh_poll util/async.c:118
> > > #6 0x562a8e5b65bd in aio_dispatch util/aio-posix.c:436
> > > #7 0x562a8e5ab26f in aio_ctx_dispatch util/async.c:261
> > > #8 0x7f687c1438ac in g_main_context_dispatch
> > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > >
> > > Thread T5 created by T0 here:
> > > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > #1 0x562a8e5bf61e in qemu_thread_create
> util/qemu-thread-posix.c:534
> > > #2 0x562a8d7a2258 in qemu_kvm_start_vcpu
> /root/qemu-3.0.0/cpus.c:1935
> > > #3 0x562a8d7a2a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > > #4 0x562a8da3ef0c in x86_cpu_realizefn
> > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > #5 0x562a8dd3a1e8 in device_set_realized hw/core/qdev.c:826
> > > #6 0x562a8e2a1e62 in property_set_bool qom/object.c:1984
> > > #7 0x562a8e29db0e in object_property_set qom/object.c:1176
> > > #8 0x562a8e2a4b00 in object_property_set_qobject
> qom/qom-qobject.c:27
> > > #9 0x562a8e29de2b in object_property_set_bool qom/object.c:1242
> > > #10 0x562a8d9ad452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > #11 0x562a8d9ad993 in pc_cpus_init
> /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > #12 0x562a8d9b79cb in pc_init1
> /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > #13 0x562a8d9b981d in pc_init_v3_0
> > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > #14 0x562a8dd4bf29 in machine_run_board_init hw/core/machine.c:830
> > > #15 0x562a8db8d46e in main /root/qemu-3.0.0/vl.c:4516
> > > #16 0x7f687133a11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> > >
> > > SUMMARY: AddressSanitizer: heap-use-after-free hw/9pfs/9p-local.c:59 in
> > > local_open_nofollow
> > > Shadow bytes around the buggy address:
> > > 0x0c0480014c90: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > 0x0c0480014ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > 0x0c0480014cb0: fa fa fa fa fa fa fd fd fa fa fd fa fa fa fd fa
> > > 0x0c0480014cc0: fa fa fa fa fa fa fa fa fa fa fd fd fa fa fd fa
> > > 0x0c0480014cd0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > =>0x0c0480014ce0: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fa fa
> > > 0x0c0480014cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > 0x0c0480014d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > 0x0c0480014d10: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > 0x0c0480014d20: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > 0x0c0480014d30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > Shadow byte legend (one shadow byte represents 8 application bytes):
> > > Addressable: 00
> > > Partially addressable: 01 02 03 04 05 06 07
> > > Heap left redzone: fa
> > > Freed heap region: fd
> > > Stack left redzone: f1
> > > Stack mid redzone: f2
> > > Stack right redzone: f3
> > > Stack after return: f5
> > > Stack use after scope: f8
> > > Global redzone: f9
> > > Global init order: f6
> > > Poisoned by user: f7
> > > Container overflow: fc
> > > Array cookie: ac
> > > Intra object redzone: bb
> > > ASan internal: fe
> > > Left alloca redzone: ca
> > > Right alloca redzone: cb
> > > ==6094==ABORTING
> > >
> > > thanks.
> > >
> > > On Mon, Nov 12, 2018 at 6:00 PM Greg Kurz <groug@kaod.org> wrote:
> > >
> > > > On Mon, 12 Nov 2018 16:28:28 +0800
> > > > zhibin hu <noirfate@gmail.com> wrote:
> > > >
> > > > > hi,
> > > > >
> > > > > i use this patch with qemu 3.0.0 and it seems not fix completely.
> > > > >
> > > > > [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> > > > > -enable-kvm -net nic,model=e1000 -net
> > > > > tap,helper=/usr/libexec/qemu-bridge-helper -hda
> > > > > /var/lib/libvirt/images/test.qcow2 -fsdev
> > > > > local,id=test_dev,path=/tmp,security_model=none -device
> > > > > virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> > > > > ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> > > > > functions and may produce false positives in some cases!
> > > > > =================================================================
> > > > > ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> > > > > 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp
> 0x7f9053444118
> > > > > READ of size 2 at 0x60200014fc50 thread T17
> > > > > #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> > > > > #1 0x7f90c8452693 in g_path_get_basename
> > > > > (/lib64/libglib-2.0.so.0+0x39693)
> > > > > #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> > > > > #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> > > > > #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> > > > > #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> > > > > #6 0x559c1c145976 in coroutine_trampoline
> > > > util/coroutine-ucontext.c:116
> > > > > #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > > (/lib64/libc.so.6+0x4c5ff)
> > > > >
> > > > > 0x60200014fc50 is located 0 bytes inside of 2-byte region
> > > > > [0x60200014fc50,0x60200014fc52)
> > > > > freed by thread T0 here:
> > > > > #0 0x7f90c90db880 in __interceptor_free
> (/lib64/libasan.so.5+0xee880)
> > > > > #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > > > #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > > > #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
> > > >
> > > > Ah, so we have a similar issue when creating files with the older
> > > > 9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
> > > >
> > > > Cheers,
> > > >
> > > > --
> > > > Greg
> > > >
> > > > > #4 0x559c1c145976 in coroutine_trampoline
> > > > util/coroutine-ucontext.c:116
> > > > > #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > > (/lib64/libc.so.6+0x4c5ff)
> > > > >
> > > > > previously allocated by thread T4 here:
> > > > > #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > > > #1 0x7f90bd743c37 in __GI___vasprintf_chk
> (/lib64/libc.so.6+0x10ac37)
> > > > >
> > > > > Thread T17 created by T16 here:
> > > > > #0 0x7f90c9038443 in pthread_create
> (/lib64/libasan.so.5+0x4b443)
> > > > > #1 0x559c1c10461e in qemu_thread_create
> util/qemu-thread-posix.c:534
> > > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > > #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> > > > > #4 0x559c1c1043e0 in qemu_thread_start
> util/qemu-thread-posix.c:504
> > > > > #5 0x7f90bd9ff593 in start_thread
> (/lib64/libpthread.so.0+0x7593)
> > > > >
> > > > > Thread T16 created by T0 here:
> > > > > #0 0x7f90c9038443 in pthread_create
> (/lib64/libasan.so.5+0x4b443)
> > > > > #1 0x559c1c10461e in qemu_thread_create
> util/qemu-thread-posix.c:534
> > > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > > #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > > > #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> > > > > #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> > > > > #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> > > > > #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> > > > > #8 0x7f90c84658ac in g_main_context_dispatch
> > > > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > > > >
> > > > > Thread T4 created by T0 here:
> > > > > #0 0x7f90c9038443 in pthread_create
> (/lib64/libasan.so.5+0x4b443)
> > > > > #1 0x559c1c10461e in qemu_thread_create
> util/qemu-thread-posix.c:534
> > > > > #2 0x559c1b2e7258 in qemu_kvm_start_vcpu
> /root/qemu-3.0.0/cpus.c:1935
> > > > > #3 0x559c1b2e7a0b in qemu_init_vcpu
> /root/qemu-3.0.0/cpus.c:2001
> > > > > #4 0x559c1b583f0c in x86_cpu_realizefn
> > > > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > > > #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> > > > > #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> > > > > #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> > > > > #8 0x559c1bde9b00 in object_property_set_qobject
> qom/qom-qobject.c:27
> > > > > #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> > > > > #10 0x559c1b4f2452 in pc_new_cpu
> /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > > > #11 0x559c1b4f2993 in pc_cpus_init
> /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > > > #12 0x559c1b4fc9cb in pc_init1
> /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > > > #13 0x559c1b4fe81d in pc_init_v3_0
> > > > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > > > #14 0x559c1b890f29 in machine_run_board_init
> hw/core/machine.c:830
> > > > > #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> > > > > #16 0x7f90bd65c11a in __libc_start_main
> (/lib64/libc.so.6+0x2311a)
> > > > >
> > > > > SUMMARY: AddressSanitizer: heap-use-after-free
> > > > > (/lib64/libasan.so.5+0x9997c)
> > > > > Shadow bytes around the buggy address:
> > > > > 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > > 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > > > 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > > > 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> > > > > 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > > =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> > > > > 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> > > > > 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> > > > > 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > > > 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > > 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > > > Shadow byte legend (one shadow byte represents 8 application
> bytes):
> > > > > Addressable: 00
> > > > > Partially addressable: 01 02 03 04 05 06 07
> > > > > Heap left redzone: fa
> > > > > Freed heap region: fd
> > > > > Stack left redzone: f1
> > > > > Stack mid redzone: f2
> > > > > Stack right redzone: f3
> > > > > Stack after return: f5
> > > > > Stack use after scope: f8
> > > > > Global redzone: f9
> > > > > Global init order: f6
> > > > > Poisoned by user: f7
> > > > > Container overflow: fc
> > > > > Array cookie: ac
> > > > > Intra object redzone: bb
> > > > > ASan internal: fe
> > > > > Left alloca redzone: ca
> > > > > Right alloca redzone: cb
> > > > > ==5014==ABORTING
> > > > >
> > > > > thanks.
> > > > >
> > > > > On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> > > > >
> > > > > > The assumption that the fid cannot be used by any other
> operation is
> > > > > > wrong. At least, nothing prevents a misbehaving client to create
> a
> > > > > > file with a given fid, and to pass this fid to some other
> operation
> > > > > > at the same time (ie, without waiting for the response to the
> creation
> > > > > > request). The call to v9fs_path_copy() performed by the worker
> thread
> > > > > > after the file was created can race with any access to the fid
> path
> > > > > > performed by some other thread. This causes use-after-free
> issues that
> > > > > > can be detected by ASAN with a custom 9p client.
> > > > > >
> > > > > > Unlike other operations that only read the fid path,
> v9fs_co_open2()
> > > > > > does modify it. It should hence take the write lock.
> > > > > >
> > > > > > Cc: P J P <ppandit@redhat.com>
> > > > > > Reported-by: zhibin hu <noirfate@gmail.com>
> > > > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > > > > ---
> > > > > > hw/9pfs/cofile.c | 6 +++---
> > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > > > > > index 88791bc327ac..9c22837cda32 100644
> > > > > > --- a/hw/9pfs/cofile.c
> > > > > > +++ b/hw/9pfs/cofile.c
> > > > > > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU
> *pdu,
> > > > > > V9fsFidState *fidp,
> > > > > > cred.fc_gid = gid;
> > > > > > /*
> > > > > > * Hold the directory fid lock so that directory path name
> > > > > > - * don't change. Read lock is fine because this fid cannot
> > > > > > - * be used by any other operation.
> > > > > > + * don't change. Take the write lock to be sure this fid
> > > > > > + * cannot be used by another operation.
> > > > > > */
> > > > > > - v9fs_path_read_lock(s);
> > > > > > + v9fs_path_write_lock(s);
> > > > > > v9fs_co_run_in_worker(
> > > > > > {
> > > > > > err = s->ops->open2(&s->ctx, &fidp->path,
> > > > > >
> > > > > >
> > > >
> > > >
> >
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()
2018-11-13 3:35 ` zhibin hu
@ 2018-11-13 8:03 ` Greg Kurz
0 siblings, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2018-11-13 8:03 UTC (permalink / raw)
To: zhibin hu; +Cc: qemu-devel, prasad
On Tue, 13 Nov 2018 11:35:23 +0800
zhibin hu <noirfate@gmail.com> wrote:
> Sorry, i have no time to make poc recently.
>
The crash log below seems to indicate you have at least one involving the
CREATE and MKNOD commands, but never mind :)
> IMHO, the implementation of v9fs_path_copy is not secure, it first free the
> original value and than copy the new value, there is a race.
>
There is a race if the first argument to v9fs_path_copy() is shared between
threads, which may be the case if it's a fid path.
> So each caller must ensure the synchronization, maybe more locks are needed.
>
Yeah.
> thanks.
>
>
> On Mon, Nov 12, 2018 at 10:39 PM Greg Kurz <groug@kaod.org> wrote:
>
> > On Mon, 12 Nov 2018 12:19:29 +0100
> > Greg Kurz <groug@kaod.org> wrote:
> >
> > > On Mon, 12 Nov 2018 19:05:59 +0800
> > > zhibin hu <noirfate@gmail.com> wrote:
> > >
> > > > yes, and this :
> > > >
> > >
> > > Yeah, all call sites of v9fs_path_copy() in v9fs_create() are called in
> > the
> > > context of the main thread. They may race with any other access to the
> > fid
> > > path performed by some other command in the context of a worker thread.
> > My
> > > first guess is that v9fs_create() should take the write lock before
> > writing
> > > to the fid path.
> > >
> >
> > I think this call to v9fs_path_copy() in v9fs_walk() can also race:
> >
> > if (fid == newfid) {
> > if (fidp->fid_type != P9_FID_NONE) {
> > err = -EINVAL;
> > goto out;
> > }
> > v9fs_path_copy(&fidp->path, &path);
> > } else {
> >
> >
> > Worse, since v9fs_co_open2() may overwrite the fid path from a worker
> > thread, it seems that some more code might require to run with the read
> > lock taken...
> >
> > > BTW, if you could share all the reproducers you already have for these
> > > heap-use-after-free issues, it would be appreciated, and probably speed
> > > up the fixing.
> > >
> > > > ==6094==ERROR: AddressSanitizer: heap-use-after-free on address
> > > > 0x6020000e6751 at pc 0x562a8dc492b8 bp 0x7f6805d2fa10 sp 0x7f6805d2fa00
> > > > READ of size 1 at 0x6020000e6751 thread T21
> > > > #0 0x562a8dc492b7 in local_open_nofollow hw/9pfs/9p-local.c:59
> > > > #1 0x562a8dc49361 in local_opendir_nofollow hw/9pfs/9p-local.c:92
> > > > #2 0x562a8dc4bd6e in local_mknod hw/9pfs/9p-local.c:662
> > > > #3 0x562a8dc521de in v9fs_co_mknod hw/9pfs/cofs.c:200
> > > > #4 0x562a8dc4413e in v9fs_mknod hw/9pfs/9p.c:3044
> > > > #5 0x562a8e600976 in coroutine_trampoline
> > util/coroutine-ucontext.c:116
> > > > #6 0x7f68713635ff in __correctly_grouped_prefixwc
> > > > (/lib64/libc.so.6+0x4c5ff)
> > > >
> > > > 0x6020000e6751 is located 1 bytes inside of 2-byte region
> > > > [0x6020000e6750,0x6020000e6752)
> > > > freed by thread T0 here:
> > > > #0 0x7f687cdb9880 in __interceptor_free
> > (/lib64/libasan.so.5+0xee880)
> > > > #1 0x7f687c1494d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > > #2 0x562a8dc30ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > > #3 0x562a8dc3f0f3 in v9fs_create hw/9pfs/9p.c:2286
> > > > #4 0x562a8e600976 in coroutine_trampoline
> > util/coroutine-ucontext.c:116
> > > > #5 0x7f68713635ff in __correctly_grouped_prefixwc
> > > > (/lib64/libc.so.6+0x4c5ff)
> > > >
> > > > previously allocated by thread T5 here:
> > > > #0 0x7f687cdb9c48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > > #1 0x7f6871421c37 in __GI___vasprintf_chk
> > (/lib64/libc.so.6+0x10ac37)
> > > >
> > > > Thread T21 created by T0 here:
> > > > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > > #1 0x562a8e5bf61e in qemu_thread_create
> > util/qemu-thread-posix.c:534
> > > > #2 0x562a8e5adbe4 in do_spawn_thread util/thread-pool.c:135
> > > > #3 0x562a8e5adc73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > > #4 0x562a8e5aa4d0 in aio_bh_call util/async.c:90
> > > > #5 0x562a8e5aa787 in aio_bh_poll util/async.c:118
> > > > #6 0x562a8e5b65bd in aio_dispatch util/aio-posix.c:436
> > > > #7 0x562a8e5ab26f in aio_ctx_dispatch util/async.c:261
> > > > #8 0x7f687c1438ac in g_main_context_dispatch
> > > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > > >
> > > > Thread T5 created by T0 here:
> > > > #0 0x7f687cd16443 in pthread_create (/lib64/libasan.so.5+0x4b443)
> > > > #1 0x562a8e5bf61e in qemu_thread_create
> > util/qemu-thread-posix.c:534
> > > > #2 0x562a8d7a2258 in qemu_kvm_start_vcpu
> > /root/qemu-3.0.0/cpus.c:1935
> > > > #3 0x562a8d7a2a0b in qemu_init_vcpu /root/qemu-3.0.0/cpus.c:2001
> > > > #4 0x562a8da3ef0c in x86_cpu_realizefn
> > > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > > #5 0x562a8dd3a1e8 in device_set_realized hw/core/qdev.c:826
> > > > #6 0x562a8e2a1e62 in property_set_bool qom/object.c:1984
> > > > #7 0x562a8e29db0e in object_property_set qom/object.c:1176
> > > > #8 0x562a8e2a4b00 in object_property_set_qobject
> > qom/qom-qobject.c:27
> > > > #9 0x562a8e29de2b in object_property_set_bool qom/object.c:1242
> > > > #10 0x562a8d9ad452 in pc_new_cpu /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > > #11 0x562a8d9ad993 in pc_cpus_init
> > /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > > #12 0x562a8d9b79cb in pc_init1
> > /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > > #13 0x562a8d9b981d in pc_init_v3_0
> > > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > > #14 0x562a8dd4bf29 in machine_run_board_init hw/core/machine.c:830
> > > > #15 0x562a8db8d46e in main /root/qemu-3.0.0/vl.c:4516
> > > > #16 0x7f687133a11a in __libc_start_main (/lib64/libc.so.6+0x2311a)
> > > >
> > > > SUMMARY: AddressSanitizer: heap-use-after-free hw/9pfs/9p-local.c:59 in
> > > > local_open_nofollow
> > > > Shadow bytes around the buggy address:
> > > > 0x0c0480014c90: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > 0x0c0480014ca0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > 0x0c0480014cb0: fa fa fa fa fa fa fd fd fa fa fd fa fa fa fd fa
> > > > 0x0c0480014cc0: fa fa fa fa fa fa fa fa fa fa fd fd fa fa fd fa
> > > > 0x0c0480014cd0: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > > =>0x0c0480014ce0: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fa fa
> > > > 0x0c0480014cf0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > > 0x0c0480014d00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > 0x0c0480014d10: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > > 0x0c0480014d20: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > > 0x0c0480014d30: fa fa fa fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > > Shadow byte legend (one shadow byte represents 8 application bytes):
> > > > Addressable: 00
> > > > Partially addressable: 01 02 03 04 05 06 07
> > > > Heap left redzone: fa
> > > > Freed heap region: fd
> > > > Stack left redzone: f1
> > > > Stack mid redzone: f2
> > > > Stack right redzone: f3
> > > > Stack after return: f5
> > > > Stack use after scope: f8
> > > > Global redzone: f9
> > > > Global init order: f6
> > > > Poisoned by user: f7
> > > > Container overflow: fc
> > > > Array cookie: ac
> > > > Intra object redzone: bb
> > > > ASan internal: fe
> > > > Left alloca redzone: ca
> > > > Right alloca redzone: cb
> > > > ==6094==ABORTING
> > > >
> > > > thanks.
> > > >
> > > > On Mon, Nov 12, 2018 at 6:00 PM Greg Kurz <groug@kaod.org> wrote:
> > > >
> > > > > On Mon, 12 Nov 2018 16:28:28 +0800
> > > > > zhibin hu <noirfate@gmail.com> wrote:
> > > > >
> > > > > > hi,
> > > > > >
> > > > > > i use this patch with qemu 3.0.0 and it seems not fix completely.
> > > > > >
> > > > > > [root@localhost ~]# ./qemu-system-x86_64 -snapshot -m 1024 -smp 2
> > > > > > -enable-kvm -net nic,model=e1000 -net
> > > > > > tap,helper=/usr/libexec/qemu-bridge-helper -hda
> > > > > > /var/lib/libvirt/images/test.qcow2 -fsdev
> > > > > > local,id=test_dev,path=/tmp,security_model=none -device
> > > > > > virtio-9p-pci,fsdev=test_dev,mount_tag=test_mount -vnc 0.0.0.0:1
> > > > > > ==5014==WARNING: ASan doesn't fully support makecontext/swapcontext
> > > > > > functions and may produce false positives in some cases!
> > > > > > =================================================================
> > > > > > ==5014==ERROR: AddressSanitizer: heap-use-after-free on address
> > > > > > 0x60200014fc50 at pc 0x7f90c908697d bp 0x7f9053444970 sp
> > 0x7f9053444118
> > > > > > READ of size 2 at 0x60200014fc50 thread T17
> > > > > > #0 0x7f90c908697c (/lib64/libasan.so.5+0x9997c)
> > > > > > #1 0x7f90c8452693 in g_path_get_basename
> > > > > > (/lib64/libglib-2.0.so.0+0x39693)
> > > > > > #2 0x559c1b78eb56 in local_lstat hw/9pfs/9p-local.c:182
> > > > > > #3 0x559c1b799628 in v9fs_co_lstat hw/9pfs/cofile.c:53
> > > > > > #4 0x559c1b777ee1 in fid_to_qid hw/9pfs/9p.c:598
> > > > > > #5 0x559c1b77aeae in v9fs_attach hw/9pfs/9p.c:1017
> > > > > > #6 0x559c1c145976 in coroutine_trampoline
> > > > > util/coroutine-ucontext.c:116
> > > > > > #7 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > > > (/lib64/libc.so.6+0x4c5ff)
> > > > > >
> > > > > > 0x60200014fc50 is located 0 bytes inside of 2-byte region
> > > > > > [0x60200014fc50,0x60200014fc52)
> > > > > > freed by thread T0 here:
> > > > > > #0 0x7f90c90db880 in __interceptor_free
> > (/lib64/libasan.so.5+0xee880)
> > > > > > #1 0x7f90c846b4d1 in g_free (/lib64/libglib-2.0.so.0+0x524d1)
> > > > > > #2 0x559c1b775ce4 in v9fs_path_copy hw/9pfs/9p.c:195
> > > > > > #3 0x559c1b78425d in v9fs_create hw/9pfs/9p.c:2297
> > > > >
> > > > > Ah, so we have a similar issue when creating files with the older
> > > > > 9p2000 and 9p2000.u protocols... I'll try to come up with a fix.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > --
> > > > > Greg
> > > > >
> > > > > > #4 0x559c1c145976 in coroutine_trampoline
> > > > > util/coroutine-ucontext.c:116
> > > > > > #5 0x7f90bd6855ff in __correctly_grouped_prefixwc
> > > > > > (/lib64/libc.so.6+0x4c5ff)
> > > > > >
> > > > > > previously allocated by thread T4 here:
> > > > > > #0 0x7f90c90dbc48 in malloc (/lib64/libasan.so.5+0xeec48)
> > > > > > #1 0x7f90bd743c37 in __GI___vasprintf_chk
> > (/lib64/libc.so.6+0x10ac37)
> > > > > >
> > > > > > Thread T17 created by T16 here:
> > > > > > #0 0x7f90c9038443 in pthread_create
> > (/lib64/libasan.so.5+0x4b443)
> > > > > > #1 0x559c1c10461e in qemu_thread_create
> > util/qemu-thread-posix.c:534
> > > > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > > > #3 0x559c1c0f2498 in worker_thread util/thread-pool.c:83
> > > > > > #4 0x559c1c1043e0 in qemu_thread_start
> > util/qemu-thread-posix.c:504
> > > > > > #5 0x7f90bd9ff593 in start_thread
> > (/lib64/libpthread.so.0+0x7593)
> > > > > >
> > > > > > Thread T16 created by T0 here:
> > > > > > #0 0x7f90c9038443 in pthread_create
> > (/lib64/libasan.so.5+0x4b443)
> > > > > > #1 0x559c1c10461e in qemu_thread_create
> > util/qemu-thread-posix.c:534
> > > > > > #2 0x559c1c0f2be4 in do_spawn_thread util/thread-pool.c:135
> > > > > > #3 0x559c1c0f2c73 in spawn_thread_bh_fn util/thread-pool.c:143
> > > > > > #4 0x559c1c0ef4d0 in aio_bh_call util/async.c:90
> > > > > > #5 0x559c1c0ef787 in aio_bh_poll util/async.c:118
> > > > > > #6 0x559c1c0fb5bd in aio_dispatch util/aio-posix.c:436
> > > > > > #7 0x559c1c0f026f in aio_ctx_dispatch util/async.c:261
> > > > > > #8 0x7f90c84658ac in g_main_context_dispatch
> > > > > > (/lib64/libglib-2.0.so.0+0x4c8ac)
> > > > > >
> > > > > > Thread T4 created by T0 here:
> > > > > > #0 0x7f90c9038443 in pthread_create
> > (/lib64/libasan.so.5+0x4b443)
> > > > > > #1 0x559c1c10461e in qemu_thread_create
> > util/qemu-thread-posix.c:534
> > > > > > #2 0x559c1b2e7258 in qemu_kvm_start_vcpu
> > /root/qemu-3.0.0/cpus.c:1935
> > > > > > #3 0x559c1b2e7a0b in qemu_init_vcpu
> > /root/qemu-3.0.0/cpus.c:2001
> > > > > > #4 0x559c1b583f0c in x86_cpu_realizefn
> > > > > > /root/qemu-3.0.0/target/i386/cpu.c:4996
> > > > > > #5 0x559c1b87f1e8 in device_set_realized hw/core/qdev.c:826
> > > > > > #6 0x559c1bde6e62 in property_set_bool qom/object.c:1984
> > > > > > #7 0x559c1bde2b0e in object_property_set qom/object.c:1176
> > > > > > #8 0x559c1bde9b00 in object_property_set_qobject
> > qom/qom-qobject.c:27
> > > > > > #9 0x559c1bde2e2b in object_property_set_bool qom/object.c:1242
> > > > > > #10 0x559c1b4f2452 in pc_new_cpu
> > /root/qemu-3.0.0/hw/i386/pc.c:1107
> > > > > > #11 0x559c1b4f2993 in pc_cpus_init
> > /root/qemu-3.0.0/hw/i386/pc.c:1155
> > > > > > #12 0x559c1b4fc9cb in pc_init1
> > /root/qemu-3.0.0/hw/i386/pc_piix.c:153
> > > > > > #13 0x559c1b4fe81d in pc_init_v3_0
> > > > > > /root/qemu-3.0.0/hw/i386/pc_piix.c:438
> > > > > > #14 0x559c1b890f29 in machine_run_board_init
> > hw/core/machine.c:830
> > > > > > #15 0x559c1b6d246e in main /root/qemu-3.0.0/vl.c:4516
> > > > > > #16 0x7f90bd65c11a in __libc_start_main
> > (/lib64/libc.so.6+0x2311a)
> > > > > >
> > > > > > SUMMARY: AddressSanitizer: heap-use-after-free
> > > > > > (/lib64/libasan.so.5+0x9997c)
> > > > > > Shadow bytes around the buggy address:
> > > > > > 0x0c0480021f30: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
> > > > > > 0x0c0480021f40: fa fa fd fa fa fa fd fd fa fa fa fa fa fa fa fa
> > > > > > 0x0c0480021f50: fa fa fa fa fa fa fa fa fa fa fd fa fa fa fd fd
> > > > > > 0x0c0480021f60: fa fa fd fa fa fa fa fa fa fa fd fa fa fa fa fa
> > > > > > 0x0c0480021f70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > > > =>0x0c0480021f80: fa fa fa fa fa fa fd fa fa fa[fd]fa fa fa fd fa
> > > > > > 0x0c0480021f90: fa fa fd fd fa fa fd fa fa fa fa fa fa fa fd fa
> > > > > > 0x0c0480021fa0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fa fa
> > > > > > 0x0c0480021fb0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fd
> > > > > > 0x0c0480021fc0: fa fa fd fa fa fa fa fa fa fa fa fa fa fa fd fa
> > > > > > 0x0c0480021fd0: fa fa fd fa fa fa fd fa fa fa fa fa fa fa fd fa
> > > > > > Shadow byte legend (one shadow byte represents 8 application
> > bytes):
> > > > > > Addressable: 00
> > > > > > Partially addressable: 01 02 03 04 05 06 07
> > > > > > Heap left redzone: fa
> > > > > > Freed heap region: fd
> > > > > > Stack left redzone: f1
> > > > > > Stack mid redzone: f2
> > > > > > Stack right redzone: f3
> > > > > > Stack after return: f5
> > > > > > Stack use after scope: f8
> > > > > > Global redzone: f9
> > > > > > Global init order: f6
> > > > > > Poisoned by user: f7
> > > > > > Container overflow: fc
> > > > > > Array cookie: ac
> > > > > > Intra object redzone: bb
> > > > > > ASan internal: fe
> > > > > > Left alloca redzone: ca
> > > > > > Right alloca redzone: cb
> > > > > > ==5014==ABORTING
> > > > > >
> > > > > > thanks.
> > > > > >
> > > > > > On Wed, Nov 7, 2018 at 8:21 AM Greg Kurz <groug@kaod.org> wrote:
> > > > > >
> > > > > > > The assumption that the fid cannot be used by any other
> > operation is
> > > > > > > wrong. At least, nothing prevents a misbehaving client to create
> > a
> > > > > > > file with a given fid, and to pass this fid to some other
> > operation
> > > > > > > at the same time (ie, without waiting for the response to the
> > creation
> > > > > > > request). The call to v9fs_path_copy() performed by the worker
> > thread
> > > > > > > after the file was created can race with any access to the fid
> > path
> > > > > > > performed by some other thread. This causes use-after-free
> > issues that
> > > > > > > can be detected by ASAN with a custom 9p client.
> > > > > > >
> > > > > > > Unlike other operations that only read the fid path,
> > v9fs_co_open2()
> > > > > > > does modify it. It should hence take the write lock.
> > > > > > >
> > > > > > > Cc: P J P <ppandit@redhat.com>
> > > > > > > Reported-by: zhibin hu <noirfate@gmail.com>
> > > > > > > Signed-off-by: Greg Kurz <groug@kaod.org>
> > > > > > > ---
> > > > > > > hw/9pfs/cofile.c | 6 +++---
> > > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > >
> > > > > > > diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
> > > > > > > index 88791bc327ac..9c22837cda32 100644
> > > > > > > --- a/hw/9pfs/cofile.c
> > > > > > > +++ b/hw/9pfs/cofile.c
> > > > > > > @@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU
> > *pdu,
> > > > > > > V9fsFidState *fidp,
> > > > > > > cred.fc_gid = gid;
> > > > > > > /*
> > > > > > > * Hold the directory fid lock so that directory path name
> > > > > > > - * don't change. Read lock is fine because this fid cannot
> > > > > > > - * be used by any other operation.
> > > > > > > + * don't change. Take the write lock to be sure this fid
> > > > > > > + * cannot be used by another operation.
> > > > > > > */
> > > > > > > - v9fs_path_read_lock(s);
> > > > > > > + v9fs_path_write_lock(s);
> > > > > > > v9fs_co_run_in_worker(
> > > > > > > {
> > > > > > > err = s->ops->open2(&s->ctx, &fidp->path,
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > >
> >
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-13 8:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-07 0:21 [Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2() Greg Kurz
2018-11-12 8:28 ` zhibin hu
2018-11-12 10:00 ` Greg Kurz
2018-11-12 11:05 ` zhibin hu
2018-11-12 11:19 ` Greg Kurz
2018-11-12 14:39 ` Greg Kurz
2018-11-13 3:35 ` zhibin hu
2018-11-13 8:03 ` Greg Kurz
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.