Skip to content

Commit

Permalink
xen/hypfs: move per-node function pointers into a dedicated struct
Browse files Browse the repository at this point in the history 
Move the function pointers currently stored in each hypfs node into a
dedicated structure in order to save some space for each node. This
will save even more space with additional callbacks added in future.

Provide some standard function vectors.

Instead of testing the write pointer to be not NULL provide a dummy
function just returning -EACCESS. ASSERT() all vector entries being
populated when adding a node. This avoids any potential problem (e.g.
pv domain privilege escalations) in case of calling a non populated
vector entry.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
  • Loading branch information
@jgross1 @jbeulich
jgross1 authored and jbeulich committed Dec 4, 2020
1 parent ba6e78f commit d290337
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 39 deletions.
41 changes: 34 additions & 7 deletions xen/common/hypfs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,27 @@ CHECK_hypfs_dirlistentry;
(DIRENTRY_NAME_OFF + \
ROUNDUP((name_len) + 1, alignof(struct xen_hypfs_direntry)))

const struct hypfs_funcs hypfs_dir_funcs = {
.read = hypfs_read_dir,
.write = hypfs_write_deny,
};
const struct hypfs_funcs hypfs_leaf_ro_funcs = {
.read = hypfs_read_leaf,
.write = hypfs_write_deny,
};
const struct hypfs_funcs hypfs_leaf_wr_funcs = {
.read = hypfs_read_leaf,
.write = hypfs_write_leaf,
};
const struct hypfs_funcs hypfs_bool_wr_funcs = {
.read = hypfs_read_leaf,
.write = hypfs_write_bool,
};
const struct hypfs_funcs hypfs_custom_wr_funcs = {
.read = hypfs_read_leaf,
.write = hypfs_write_custom,
};

static DEFINE_RWLOCK(hypfs_lock);
enum hypfs_lock_state {
hypfs_unlocked,
Expand Down Expand Up @@ -74,6 +95,9 @@ static int add_entry(struct hypfs_entry_dir *parent, struct hypfs_entry *new)
int ret = -ENOENT;
struct hypfs_entry *e;

ASSERT(new->funcs->read);
ASSERT(new->funcs->write);

hypfs_write_lock();

list_for_each_entry ( e, &parent->dirlist, list )
Expand Down Expand Up @@ -284,7 +308,7 @@ static int hypfs_read(const struct hypfs_entry *entry,

guest_handle_add_offset(uaddr, sizeof(e));

ret = entry->read(entry, uaddr);
ret = entry->funcs->read(entry, uaddr);

out:
return ret;
Expand All @@ -297,6 +321,7 @@ int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
int ret;

ASSERT(this_cpu(hypfs_locked) == hypfs_write_locked);
ASSERT(leaf->e.max_size);

if ( ulen > leaf->e.max_size )
return -ENOSPC;
Expand Down Expand Up @@ -357,6 +382,7 @@ int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
int ret;

ASSERT(this_cpu(hypfs_locked) == hypfs_write_locked);
ASSERT(leaf->e.max_size);

/* Avoid oversized buffer allocation. */
if ( ulen > MAX_PARAM_SIZE )
Expand All @@ -382,19 +408,20 @@ int hypfs_write_custom(struct hypfs_entry_leaf *leaf,
return ret;
}

int hypfs_write_deny(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned int ulen)
{
return -EACCES;
}

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;

ASSERT(entry->max_size);

l = container_of(entry, struct hypfs_entry_leaf, e);

return entry->write(l, uaddr, ulen);
return entry->funcs->write(l, uaddr, ulen);
}

long do_hypfs_op(unsigned int cmd,
Expand Down
71 changes: 49 additions & 22 deletions xen/include/xen/hypfs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,32 @@
#include <public/hypfs.h>

struct hypfs_entry_leaf;
struct hypfs_entry;

/*
* Per-node callbacks:
*
* The callbacks are always called with the hypfs lock held.
*
* The read() callback is used to return the contents of a node (either
* directory or leaf). It is NOT used to get directory entries during traversal
* of the tree.
*
* The write() callback is used to modify the contents of a node. Writing
* directories is not supported (this means all nodes are added at boot time).
*/
struct hypfs_funcs {
int (*read)(const struct hypfs_entry *entry,
XEN_GUEST_HANDLE_PARAM(void) uaddr);
int (*write)(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned int ulen);
};

extern const struct hypfs_funcs hypfs_dir_funcs;
extern const struct hypfs_funcs hypfs_leaf_ro_funcs;
extern const struct hypfs_funcs hypfs_leaf_wr_funcs;
extern const struct hypfs_funcs hypfs_bool_wr_funcs;
extern const struct hypfs_funcs hypfs_custom_wr_funcs;

struct hypfs_entry {
unsigned short type;
Expand All @@ -15,10 +41,7 @@ struct hypfs_entry {
unsigned int max_size;
const char *name;
struct list_head list;
int (*read)(const struct hypfs_entry *entry,
XEN_GUEST_HANDLE_PARAM(void) uaddr);
int (*write)(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned int ulen);
const struct hypfs_funcs *funcs;
};

struct hypfs_entry_leaf {
Expand All @@ -42,7 +65,7 @@ struct hypfs_entry_dir {
.e.size = 0, \
.e.max_size = 0, \
.e.list = LIST_HEAD_INIT(var.e.list), \
.e.read = hypfs_read_dir, \
.e.funcs = &hypfs_dir_funcs, \
.dirlist = LIST_HEAD_INIT(var.dirlist), \
}

Expand All @@ -52,7 +75,7 @@ struct hypfs_entry_dir {
.e.encoding = XEN_HYPFS_ENC_PLAIN, \
.e.name = (nam), \
.e.max_size = (msz), \
.e.read = hypfs_read_leaf, \
.e.funcs = &hypfs_leaf_ro_funcs, \
}

/* Content and size need to be set via hypfs_string_set_reference(). */
Expand All @@ -72,35 +95,37 @@ static inline void hypfs_string_set_reference(struct hypfs_entry_leaf *leaf,
leaf->e.size = strlen(str) + 1;
}

#define HYPFS_FIXEDSIZE_INIT(var, typ, nam, contvar, wr) \
struct hypfs_entry_leaf __read_mostly var = { \
.e.type = (typ), \
.e.encoding = XEN_HYPFS_ENC_PLAIN, \
.e.name = (nam), \
.e.size = sizeof(contvar), \
.e.max_size = (wr) ? sizeof(contvar) : 0, \
.e.read = hypfs_read_leaf, \
.e.write = (wr), \
.u.content = &(contvar), \
#define HYPFS_FIXEDSIZE_INIT(var, typ, nam, contvar, fn, wr) \
struct hypfs_entry_leaf __read_mostly var = { \
.e.type = (typ), \
.e.encoding = XEN_HYPFS_ENC_PLAIN, \
.e.name = (nam), \
.e.size = sizeof(contvar), \
.e.max_size = (wr) ? sizeof(contvar) : 0, \
.e.funcs = (fn), \
.u.content = &(contvar), \
}

#define HYPFS_UINT_INIT(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_UINT, nam, contvar, NULL)
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_UINT, nam, contvar, \
&hypfs_leaf_ro_funcs, 0)
#define HYPFS_UINT_INIT_WRITABLE(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_UINT, nam, contvar, \
hypfs_write_leaf)
&hypfs_leaf_wr_funcs, 1)

#define HYPFS_INT_INIT(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_INT, nam, contvar, NULL)
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_INT, nam, contvar, \
&hypfs_leaf_ro_funcs, 0)
#define HYPFS_INT_INIT_WRITABLE(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_INT, nam, contvar, \
hypfs_write_leaf)
&hypfs_leaf_wr_funcs, 1)

#define HYPFS_BOOL_INIT(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_BOOL, nam, contvar, NULL)
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_BOOL, nam, contvar, \
&hypfs_leaf_ro_funcs, 0)
#define HYPFS_BOOL_INIT_WRITABLE(var, nam, contvar) \
HYPFS_FIXEDSIZE_INIT(var, XEN_HYPFS_TYPE_BOOL, nam, contvar, \
hypfs_write_bool)
&hypfs_bool_wr_funcs, 1)

extern struct hypfs_entry_dir hypfs_root;

Expand All @@ -112,6 +137,8 @@ int hypfs_read_dir(const struct hypfs_entry *entry,
XEN_GUEST_HANDLE_PARAM(void) uaddr);
int hypfs_read_leaf(const struct hypfs_entry *entry,
XEN_GUEST_HANDLE_PARAM(void) uaddr);
int hypfs_write_deny(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned int ulen);
int hypfs_write_leaf(struct hypfs_entry_leaf *leaf,
XEN_GUEST_HANDLE_PARAM(void) uaddr, unsigned int ulen);
int hypfs_write_bool(struct hypfs_entry_leaf *leaf,
Expand Down
15 changes: 5 additions & 10 deletions xen/include/xen/param.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,7 @@ extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
{ .hypfs.e.type = XEN_HYPFS_TYPE_STRING, \
.hypfs.e.encoding = XEN_HYPFS_ENC_PLAIN, \
.hypfs.e.name = (nam), \
.hypfs.e.read = hypfs_read_leaf, \
.hypfs.e.write = hypfs_write_custom, \
.hypfs.e.funcs = &hypfs_custom_wr_funcs, \
.init_leaf = (initfunc), \
.func = (variable) }
#define boolean_runtime_only_param(nam, variable) \
Expand All @@ -127,8 +126,7 @@ extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
.hypfs.e.name = (nam), \
.hypfs.e.size = sizeof(variable), \
.hypfs.e.max_size = sizeof(variable), \
.hypfs.e.read = hypfs_read_leaf, \
.hypfs.e.write = hypfs_write_bool, \
.hypfs.e.funcs = &hypfs_bool_wr_funcs, \
.hypfs.u.content = &(variable) }
#define integer_runtime_only_param(nam, variable) \
__paramfs __parfs_##variable = \
Expand All @@ -137,8 +135,7 @@ extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
.hypfs.e.name = (nam), \
.hypfs.e.size = sizeof(variable), \
.hypfs.e.max_size = sizeof(variable), \
.hypfs.e.read = hypfs_read_leaf, \
.hypfs.e.write = hypfs_write_leaf, \
.hypfs.e.funcs = &hypfs_leaf_wr_funcs, \
.hypfs.u.content = &(variable) }
#define size_runtime_only_param(nam, variable) \
__paramfs __parfs_##variable = \
Expand All @@ -147,8 +144,7 @@ extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
.hypfs.e.name = (nam), \
.hypfs.e.size = sizeof(variable), \
.hypfs.e.max_size = sizeof(variable), \
.hypfs.e.read = hypfs_read_leaf, \
.hypfs.e.write = hypfs_write_leaf, \
.hypfs.e.funcs = &hypfs_leaf_wr_funcs, \
.hypfs.u.content = &(variable) }
#define string_runtime_only_param(nam, variable) \
__paramfs __parfs_##variable = \
Expand All @@ -157,8 +153,7 @@ extern struct param_hypfs __paramhypfs_start[], __paramhypfs_end[];
.hypfs.e.name = (nam), \
.hypfs.e.size = 0, \
.hypfs.e.max_size = sizeof(variable), \
.hypfs.e.read = hypfs_read_leaf, \
.hypfs.e.write = hypfs_write_leaf, \
.hypfs.e.funcs = &hypfs_leaf_wr_funcs, \
.hypfs.u.content = &(variable) }

#else
Expand Down

0 comments on commit d290337

Please sign in to comment.