[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 04/12] xen: add basic hypervisor filesystem support
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Jürgen Groß <jgross@xxxxxxxx>
- Date: Fri, 3 Apr 2020 17:33:41 +0200
- Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Fri, 03 Apr 2020 15:33:45 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 03.04.20 17:31, Jan Beulich wrote:
On 03.04.2020 17:05, Jürgen Groß wrote:
On 03.04.20 16:23, Jan Beulich wrote:
On 02.04.2020 17:46, Juergen Gross wrote:
+int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ char *buf;
+ int ret;
+
+ if ( leaf->e.type != XEN_HYPFS_TYPE_STRING &&
+ leaf->e.type != XEN_HYPFS_TYPE_BLOB && ulen != leaf->e.size )
+ return -EDOM;
+
+ buf = xmalloc_array(char, ulen);
+ if ( !buf )
+ return -ENOMEM;
+
+ ret = -EFAULT;
+ if ( copy_from_guest(buf, uaddr, ulen) )
+ goto out;
+
+ ret = -EINVAL;
+ if ( leaf->e.type == XEN_HYPFS_TYPE_STRING &&
+ memchr(buf, 0, ulen) != (buf + ulen - 1) )
+ goto out;
+
+ ret = 0;
+ memcpy(leaf->write_ptr, buf, ulen);
+ leaf->e.size = ulen;
+
+ out:
+ xfree(buf);
+ return ret;
+}
+
+int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ bool buf;
+
+ ASSERT(leaf->e.type == XEN_HYPFS_TYPE_BOOL && leaf->e.size ==
sizeof(bool));
+
+ if ( ulen != leaf->e.max_size )
Why max_size here when the ASSERT() checks size?
Just for consistency with the other write functions.
In which case perhaps extend the ASSERT() to also check max_size?
Okay.
+static int hypfs_write(struct hypfs_entry *entry,
+ XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned long ulen)
+{
+ struct hypfs_entry_leaf *l;
+
+ if ( !entry->write )
+ return -EACCES;
+
+ if ( ulen > entry->max_size )
+ return -ENOSPC;
max_size being zero for non-writable entries, perhaps use -EACCES
also for this special case? Together with the other comment above,
maybe the ->write check wants replacing this way?
Checking the write function being not NULL is a nice security addon,
as I avoid to call into a non existing function. Basically both tests
would be equivalent, but this one is IMO better to avoid crashes.
In which case perhaps ASSERT(entry->max_size) between the two if()s?
Okay.
Juergen
|
