- 02 5月, 2018 1 次提交
-
-
由 Greg Kurz 提交于
Don't print the tv_nsec part of atime and mtime, to stay below the 10 argument limit of trace events. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
-
- 20 2月, 2018 2 次提交
-
-
由 Marc-André Lureau 提交于
Leak found thanks to ASAN: Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x55995789ac90 in __interceptor_malloc (/home/elmarco/src/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x1510c90) #1 0x7f0a91190f0c in g_malloc /home/elmarco/src/gnome/glib/builddir/../glib/gmem.c:94 #2 0x5599580a281c in v9fs_path_copy /home/elmarco/src/qemu/hw/9pfs/9p.c:196:17 #3 0x559958f9ec5d in coroutine_trampoline /home/elmarco/src/qemu/util/coroutine-ucontext.c:116:9 #4 0x7f0a8766ebbf (/lib64/libc.so.6+0x50bbf) Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Marc-André Lureau 提交于
lhs/rhs doesn't tell much about how argument are handled, dst/src is and const arguments is clearer in my mind. Use g_memdup() while at it. Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com> Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 09 2月, 2018 2 次提交
-
-
由 Markus Armbruster 提交于
qemu-common.h includes qemu/option.h, but most places that include the former don't actually need the latter. Drop the include, and add it to the places that actually need it. While there, drop superfluous includes of both headers, and separate #include from file comment with a blank line. This cleanup makes the number of objects depending on qemu/option.h drop from 4545 (out of 4743) to 284 in my "build everything" tree. Reviewed-by: NEric Blake <eblake@redhat.com> Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: NMarkus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-20-armbru@redhat.com> [Semantic conflict with commit bdd6a90a in block/nvme.c resolved]
-
由 Markus Armbruster 提交于
This cleanup makes the number of objects depending on qapi/error.h drop from 1910 (out of 4743) to 1612 in my "build everything" tree. While there, separate #include from file comment with a blank line, and drop a useless comment on why qemu/osdep.h is included first. Reviewed-by: NEric Blake <eblake@redhat.com> Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: NMarkus Armbruster <armbru@redhat.com> Message-Id: <20180201111846.21846-5-armbru@redhat.com> [Semantic conflict with commit 34e304e9 resolved, OSX breakage fixed]
-
- 02 2月, 2018 6 次提交
-
-
由 Greg Kurz 提交于
The idea is to send a victim request that will possibly block in the server and to send a flush request to cancel the victim request. This patch adds two test to verifiy that: - the server does not reply to a victim request that was actually cancelled - the server replies to the flush request after replying to the victim request if it could not cancel it 9p request cancellation reference: http://man.cat-v.org/plan_9/5/flushSigned-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com> (groug, change the test to only write a single byte to avoid any alignment or endianess consideration)
-
由 Greg Kurz 提交于
Trivial test of a successful write. Signed-off-by: NGreg Kurz <groug@kaod.org> (groug, handle potential overflow when computing request size, add missing g_free(buf), backend handles one written byte at a time to validate the server doesn't do short-reads) Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
-
由 Greg Kurz 提交于
Trivial test of a successful open. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
-
由 Greg Kurz 提交于
The purpose of virtio-9p-test is to test the virtio-9p device, especially the 9p server state machine. We don't really care what fsdev backend we're using. Moreover, if we want to be able to test the flush request or a device reset with in-flights I/O, it is close to impossible to achieve with a physical backend because we cannot ask it reliably to put an I/O on hold at a specific point in time. Fortunately, we can do that with the synthetic backend, which allows to register callbacks on read/write accesses to a specific file. This will be used by a later patch to test the 9P flush request. The walk request test is converted to using the synth backend. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
-
由 Keno Fischer 提交于
# Background I was investigating spurious non-deterministic EINTR returns from various 9p file system operations in a Linux guest served from the qemu 9p server. ## EINTR, ERESTARTSYS and the linux kernel When a signal arrives that the Linux kernel needs to deliver to user-space while a given thread is blocked (in the 9p case waiting for a reply to its request in 9p_client_rpc -> wait_event_interruptible), it asks whatever driver is currently running to abort its current operation (in the 9p case causing the submission of a TFLUSH message) and return to user space. In these situations, the error message reported is generally ERESTARTSYS. If the userspace processes specified SA_RESTART, this means that the system call will get restarted upon completion of the signal handler delivery (assuming the signal handler doesn't modify the process state in complicated ways not relevant here). If SA_RESTART is not specified, ERESTARTSYS gets translated to EINTR and user space is expected to handle the restart itself. ## The 9p TFLUSH command The 9p TFLUSH commands requests that the server abort an ongoing operation. The man page [1] specifies: ``` If it recognizes oldtag as the tag of a pending transaction, it should abort any pending response and discard that tag. [...] When the client sends a Tflush, it must wait to receive the corresponding Rflush before reusing oldtag for subsequent messages. If a response to the flushed request is received before the Rflush, the client must honor the response as if it had not been flushed, since the completed request may signify a state change in the server ``` In particular, this means that the server must not send a reply with the orignal tag in response to the cancellation request, because the client is obligated to interpret such a reply as a coincidental reply to the original request. # The bug When qemu receives a TFlush request, it sets the `cancelled` flag on the relevant pdu. This flag is periodically checked, e.g. in `v9fs_co_name_to_path`, and if set, the operation is aborted and the error is set to EINTR. However, the server then violates the spec, by returning to the client an Rerror response, rather than discarding the message entirely. As a result, the client is required to assume that said Rerror response is a result of the original request, not a result of the cancellation and thus passes the EINTR error back to user space. This is not the worst thing it could do, however as discussed above, the correct error code would have been ERESTARTSYS, such that user space programs with SA_RESTART set get correctly restarted upon completion of the signal handler. Instead, such programs get spurious EINTR results that they were not expecting to handle. It should be noted that there are plenty of user space programs that do not set SA_RESTART and do not correctly handle EINTR either. However, that is then a userspace bug. It should also be noted that this bug has been mitigated by a recent commit to the Linux kernel [2], which essentially prevents the kernel from sending Tflush requests unless the process is about to die (in which case the process likely doesn't care about the response). Nevertheless, for older kernels and to comply with the spec, I believe this change is beneficial. # Implementation The fix is fairly simple, just skipping notification of a reply if the pdu was previously cancelled. We do however, also notify the transport layer that we're doing this, so it can clean up any resources it may be holding. I also added a new trace event to distinguish operations that caused an error reply from those that were cancelled. One complication is that we only omit sending the message on EINTR errors in order to avoid confusing the rest of the code (which may assume that a client knows about a fid if it sucessfully passed it off to pud_complete without checking for cancellation status). This does mean that if the server acts upon the cancellation flag, it always needs to set err to EINTR. I believe this is true of the current code. [1] https://9fans.github.io/plan9port/man/man9/flush.html [2] https://github.com/torvalds/linux/commit/9523feac272ccad2ad8186ba4fcc891Signed-off-by: NKeno Fischer <keno@juliacomputing.com> Reviewed-by: NGreg Kurz <groug@kaod.org> [groug, send a zero-sized reply instead of detaching the buffer] Signed-off-by: NGreg Kurz <groug@kaod.org> Acked-by: NMichael S. Tsirkin <mst@redhat.com> Reviewed-by: NStefano Stabellini <sstabellini@kernel.org>
-
由 Greg Kurz 提交于
No good reasons to do this outside of v9fs_device_realize_common(). Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NStefano Stabellini <sstabellini@kernel.org>
-
- 08 1月, 2018 10 次提交
-
-
由 Greg Kurz 提交于
This backend raise some concerns: - doesn't support symlinks - fails +100 tests in the PJD POSIX file system test suite [1] - requires the QEMU process to run with the CAP_DAC_READ_SEARCH capability, which isn't recommended for security reasons This backend should not be used and wil be removed. The 'local' backend is the recommended alternative. [1] https://www.tuxera.com/community/posix-test-suite/Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NDaniel P. Berrange <berrange@redhat.com> Reviewed-by: NAneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
-
由 Greg Kurz 提交于
This patch changes some error messages in the backend init code and convert backends to propagate QEMU Error objects instead of calling error_report(). One notable improvement is that the local backend now provides a more detailed error report when it fails to open the shared directory. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
This patch changes some error messages in the backend opts parsing code and convert backends to propagate QEMU Error objects instead of calling error_report(). Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
They're only used by the 9p core code. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NEric Blake <eblake@redhat.com>
-
由 Greg Kurz 提交于
If we receive an unsupported request id, we first decide to return -ENOTSUPP to the client, but since the request id causes is_read_only_op() to return false, we change the error to be -EROFS if the fsdev is read-only. This doesn't make sense since we don't know what the client asked for. This patch ensures that -EROFS can only be returned if the request id is supported. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NEric Blake <eblake@redhat.com>
-
由 Greg Kurz 提交于
To comply with the QEMU coding style. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NEric Blake <eblake@redhat.com>
-
由 Greg Kurz 提交于
To comply with the QEMU coding style. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
To comply with the QEMU coding style. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
To comply with the QEMU coding style. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
And drop the now useless forward declaration of virtio_9p_transport. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 07 11月, 2017 2 次提交
-
-
由 Greg Kurz 提交于
The return value of v9fs_mark_fids_unreclaim() is then propagated to pdu_complete(). It should be a negative errno, not -1. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NEric Blake <eblake@redhat.com>
-
由 Greg Kurz 提交于
To comply with QEMU coding style. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 16 10月, 2017 1 次提交
-
-
由 Prasad J Pandit 提交于
9p back-end first queries the size of an extended attribute, allocates space for it via g_malloc() and then retrieves its value into allocated buffer. Race between querying attribute size and retrieving its could lead to memory bytes disclosure. Use g_malloc0() to avoid it. Reported-by: NTuomas Tynkkynen <tuomas.tynkkynen@iki.fi> Signed-off-by: NPrasad J Pandit <pjp@fedoraproject.org> Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 20 9月, 2017 3 次提交
-
-
由 Jan Dakinevich 提交于
v9fs_do_readdir_with_stat() should check for a maximum buffer size before an attempt to marshal gathered data. Otherwise, buffers assumed as misconfigured and the transport would be broken. The patch brings v9fs_do_readdir_with_stat() in conformity with v9fs_do_readdir() behavior. Signed-off-by: NJan Dakinevich <jan.dakinevich@gmail.com> [groug, regression caused my commit 8d37de41 # 2.10] Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Jan Dakinevich 提交于
The third parameter of v9fs_co_name_to_path() must not contain `/' character. The issue is most likely related to 9p2000.u protocol only. Signed-off-by: NJan Dakinevich <jan.dakinevich@gmail.com> [groug, regression caused by commit f57f5878 # 2.10] Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Jan Dakinevich 提交于
If the client is using 9p2000.u, the following occurs: $ cd ${virtfs_shared_dir} $ mkdir -p a/b/c $ ls a/b ls: cannot access 'a/b/a': No such file or directory ls: cannot access 'a/b/b': No such file or directory a b c instead of the expected: $ ls a/b c This is a regression introduced by commit f57f5878; local_name_to_path() now resolves ".." and "." in paths, and v9fs_do_readdir_with_stat()->stat_to_v9stat() then copies the basename of the resulting path to the response. With the example above, this means that "." and ".." are turned into "b" and "a" respectively... stat_to_v9stat() currently assumes it is passed a full canonicalized path and uses it to do two different things: 1) to pass it to v9fs_co_readlink() in case the file is a symbolic link 2) to set the name field of the V9fsStat structure to the basename part of the given path It only has two users: v9fs_stat() and v9fs_do_readdir_with_stat(). v9fs_stat() really needs 1) and 2) to be performed since it starts with the full canonicalized path stored in the fid. It is different for v9fs_do_readdir_with_stat() though because the name we want to put into the V9fsStat structure is the d_name field of the dirent actually (ie, we want to keep the "." and ".." special names). So, we only need 1) in this case. This patch hence adds a basename argument to stat_to_v9stat(), to be used to set the name field of the V9fsStat structure, and moves the basename logic to v9fs_stat(). Signed-off-by: NJan Dakinevich <jan.dakinevich@gmail.com> (groug, renamed old name argument to path and updated changelog) Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 05 9月, 2017 2 次提交
-
-
由 Greg Kurz 提交于
Since fchmodat(2) on Linux doesn't support AT_SYMLINK_NOFOLLOW, we have to implement it using workarounds. There are two different ways, depending on whether the system supports O_PATH or not. In the case O_PATH is supported, we rely on the behavhior of openat(2) when passing O_NOFOLLOW | O_PATH and the file is a symbolic link. Even if openat_file() already adds O_NOFOLLOW to the flags, this patch makes it explicit that we need both creation flags to obtain the expected behavior. This is only cleanup, no functional change. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
-
由 Philippe Mathieu-Daudé 提交于
(note this is how other functions also handle the errors). hw/9pfs/9p.c:948:18: warning: Loss of sign in implicit conversion offset = err; ^~~ Reported-by: Clang Static Analyzer Signed-off-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: NGreg Kurz <groug@kaod.org>
-
- 31 8月, 2017 1 次提交
-
-
由 Cornelia Huck 提交于
Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend on CONFIG_VIRTFS and CONFIG_VIRTIO/CONFIG_XEN only. Acked-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NThomas Huth <thuth@redhat.com> Acked-by: NChristian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: NCornelia Huck <cohuck@redhat.com>
-
- 10 8月, 2017 1 次提交
-
-
由 Greg Kurz 提交于
This function has to ensure it doesn't follow a symlink that could be used to escape the virtfs directory. This could be easily achieved if fchmodat() on linux honored the AT_SYMLINK_NOFOLLOW flag as described in POSIX, but it doesn't. There was a tentative to implement a new fchmodat2() syscall with the correct semantics: https://patchwork.kernel.org/patch/9596301/ but it didn't gain much momentum. Also it was suggested to look at an O_PATH based solution in the first place. The current implementation covers most use-cases, but it notably fails if: - the target path has access rights equal to 0000 (openat() returns EPERM), => once you've done chmod(0000) on a file, you can never chmod() again - the target path is UNIX domain socket (openat() returns ENXIO) => bind() of UNIX domain sockets fails if the file is on 9pfs The solution is to use O_PATH: openat() now succeeds in both cases, and we can ensure the path isn't a symlink with fstat(). The associated entry in "/proc/self/fd" can hence be safely passed to the regular chmod() syscall. The previous behavior is kept for older systems that don't have O_PATH. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NEric Blake <eblake@redhat.com> Tested-by: NZhi Yong Wu <zhiyong.wu@ucloud.cn> Acked-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org>
-
- 31 7月, 2017 1 次提交
-
-
由 Philippe Mathieu-Daudé 提交于
With the move of some docs/ to docs/devel/ on ac06724a, no references were updated. Signed-off-by: NPhilippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com> Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>
-
- 13 7月, 2017 1 次提交
-
-
由 Alistair Francis 提交于
Convert all uses of error_report("warning:"... to use warn_report() instead. This helps standardise on a single method of printing warnings to the user. All of the warnings were changed using these two commands: find ./* -type f -exec sed -i \ 's|error_report(".*warning[,:] |warn_report("|Ig' {} + Indentation fixed up manually afterwards. The test-qdev-global-props test case was manually updated to ensure that this patch passes make check (as the test cases are case sensitive). Signed-off-by: NAlistair Francis <alistair.francis@xilinx.com> Suggested-by: NThomas Huth <thuth@redhat.com> Cc: Jeff Cody <jcody@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Max Reitz <mreitz@redhat.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Peter Lieven <pl@kamp.de> Cc: Josh Durgin <jdurgin@redhat.com> Cc: "Richard W.M. Jones" <rjones@redhat.com> Cc: Markus Armbruster <armbru@redhat.com> Cc: Peter Crosthwaite <crosthwaite.peter@gmail.com> Cc: Richard Henderson <rth@twiddle.net> Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com> Cc: Greg Kurz <groug@kaod.org> Cc: Rob Herring <robh@kernel.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Peter Chubb <peter.chubb@nicta.com.au> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: Marcel Apfelbaum <marcel@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: David Gibson <david@gibson.dropbear.id.au> Cc: Alexander Graf <agraf@suse.de> Cc: Gerd Hoffmann <kraxel@redhat.com> Cc: Jason Wang <jasowang@redhat.com> Cc: Marcelo Tosatti <mtosatti@redhat.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: Cornelia Huck <cohuck@redhat.com> Cc: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: NDavid Gibson <david@gibson.dropbear.id.au> Acked-by: NGreg Kurz <groug@kaod.org> Acked-by: NCornelia Huck <cohuck@redhat.com> Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com> Reviewed by: Peter Chubb <peter.chubb@data61.csiro.au> Acked-by: NMax Reitz <mreitz@redhat.com> Acked-by: NMarcel Apfelbaum <marcel@redhat.com> Message-Id: <e1cfa2cd47087c248dd24caca9c33d9af0c499b0.1499866456.git.alistair.francis@xilinx.com> Reviewed-by: NMarkus Armbruster <armbru@redhat.com> Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
-
- 29 6月, 2017 7 次提交
-
-
由 Greg Kurz 提交于
Contrary to what is written in the comment, a buggy guest can misconfigure the transport buffers and pdu_marshal() may return an error. If this ever happens, it is up to the transport layer to handle the situation (9P is transport agnostic). This fixes Coverity issue CID1348518. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NStefano Stabellini <sstabellini@kernel.org>
-
由 Stefano Stabellini 提交于
Implement xen_9pfs_disconnect by unbinding the event channels. On xen_9pfs_free, call disconnect if any event channels haven't been disconnected. If the frontend misconfigured the buffers set the backend to "Closing" and disconnect it. Misconfigurations include requesting a read of more bytes than available on the ring buffer, or claiming to be writing more data than available on the ring buffer. Signed-off-by: NStefano Stabellini <stefano@aporeto.com> Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
The 9P protocol is transport agnostic: if the guest misconfigured the buffers, the best we can do is to set the broken flag on the device. Signed-off-by: NGreg Kurz <groug@kaod.org>
-
由 Greg Kurz 提交于
The 9p spec at http://man.cat-v.org/plan_9/5/intro reads: "Each 9P message begins with a four-byte size field specify- ing the length in bytes of the complete message including the four bytes of the size field itself. The next byte is the message type, one of the constants in the enumeration in the include file <fcall.h>. The next two bytes are an iden- tifying tag, described below." ie, each message starts with a 7-byte long header. The core 9P code already assumes this pretty much everywhere. This patch does the following: - makes the assumption explicit in the common 9p.h header, since it isn't related to the transport - open codes the header size in handle_9p_output() and hardens the sanity check on the space needed for the reply message Signed-off-by: NGreg Kurz <groug@kaod.org> Acked-by: NStefano Stabellini <sstabellini@kernel.org>
-
由 Greg Kurz 提交于
If the guest sends a malformed request, we end up with a dangling pointer in V9fsVirtioState. This doesn't seem to cause any bug, but let's remove this side effect anyway. Signed-off-by: NGreg Kurz <groug@kaod.org> Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>
-
由 Marc-André Lureau 提交于
I found these pattern via grepping the source tree. I don't have a coccinelle script for it! Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
-
由 Tobias Schramm 提交于
In mapped security modes, files are created with very restrictive permissions (600 for files and 700 for directories). This makes file sharing between virtual machines and users on the host rather complicated. Imagine eg. a group of users that need to access data produced by processes on a virtual machine. Giving those users access to the data will be difficult since the group access mode is always 0. This patch makes the default mode for both files and directories configurable. Existing setups that don't know about the new parameters keep using the current secure behavior. Signed-off-by: NTobias Schramm <tobleminer@gmail.com> Signed-off-by: NGreg Kurz <groug@kaod.org>
-