Page MenuHomeFreeBSD
Differential D35883 Diff 109621 sys/compat/lindebugfs/lindebugfs.c

Changeset View

Standalone View

View Options

sys/compat/lindebugfs/lindebugfs.c

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines
#include <machine/bus.h> #include <machine/bus.h>
#include <compat/linux/linux_ioctl.h> #include <compat/linux/linux_ioctl.h>
#include <compat/linux/linux_mib.h> #include <compat/linux/linux_mib.h>
#include <compat/linux/linux_util.h> #include <compat/linux/linux_util.h>
#include <fs/pseudofs/pseudofs.h> #include <fs/pseudofs/pseudofs.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
#include <linux/compat.h> #include <linux/compat.h>
#include <linux/debugfs.h>
#include <linux/fs.h>
MALLOC_DEFINE(M_DFSINT, "debugfsint", "Linux debugfs internal"); MALLOC_DEFINE(M_DFSINT, "debugfsint", "Linux debugfs internal");
static struct pfs_node *debugfs_root; static struct pfs_node *debugfs_root;
#define DM_SYMLINK 0x1 #define DM_SYMLINK 0x1
#define DM_DIR 0x2 #define DM_DIR 0x2
#define DM_FILE 0x3 #define DM_FILE 0x3
Show All 30 Linesdebugfs_destroy(PFS_DESTROY_ARGS)
return (0); return (0);
} }
static int static int
debugfs_fill(PFS_FILL_ARGS) debugfs_fill(PFS_FILL_ARGS)
{ {
struct dentry_meta *d; struct dentry_meta *d;
struct linux_file lf = {}; struct linux_file lf = {};
struct seq_file *sf;
struct vnode vn; struct vnode vn;
void *buf; char buf[sb->s_size];
markjUnsubmitted
Done
Inline Actions

Declaring variable-length arrays on the stack is not a good idea. Kernel threads have limited stack space and only primitive means of detecting stack overflow (one guard page at the end of the stack). You should malloc() a temporary buffer instead.

markj: Declaring variable-length arrays on the stack is not a good idea. Kernel threads have limited…
int rc; int rc;
size_t len; off_t off = 0;
off_t off;
d = pn->pn_data;
if ((rc = linux_set_current_flags(curthread, M_NOWAIT))) if ((rc = linux_set_current_flags(curthread, M_NOWAIT)))
return (rc); return (rc);
d = pn->pn_data;
vn.v_data = d->dm_data; vn.v_data = d->dm_data;
if (uio->uio_rw == UIO_READ) {
buf = uio->uio_iov[0].iov_base; /* seq_file requires sbuf for r/w */
len = min(uio->uio_iov[0].iov_len, uio->uio_resid); lf.private_data = (void *) sb;
} else {
sbuf_finish(sb);
buf = sbuf_data(sb);
len = sbuf_len(sb);
}
off = 0;
rc = d->dm_fops->open(&vn, &lf); rc = d->dm_fops->open(&vn, &lf);
if (rc < 0) { if (rc < 0) {
#ifdef INVARIANTS #ifdef INVARIANTS
printf("%s:%d open failed with %d\n", __FUNCTION__, __LINE__, rc); printf("%s:%d open failed with %d\n", __FUNCTION__, __LINE__, rc);
#endif #endif
return (-rc); return (-rc);
} }
sf = lf.private_data;
sf->buf = sb;
if (uio->uio_rw == UIO_READ) {
if (d->dm_fops->read)
rc = d->dm_fops->read(&lf, NULL, len, &off);
else
rc = -ENODEV; rc = -ENODEV;
} else { if (uio->uio_rw == UIO_READ && d->dm_fops->read) {
if (d->dm_fops->write) buf[0] = '\0';
hselaskyUnsubmitted
Done
Inline Actions

if sb->s_size == 0
return (-EINVAL);

or

MPASS(sb->s_size != 0);

hselasky: if sb->s_size == 0 return (-EINVAL); or MPASS(sb->s_size != 0);
jfreeAuthorUnsubmitted
Done
Inline Actions

The buf[0] = '\0' exists for situations where nothing is written into buf. A good example is the seq_file ->read() implementation which uses seq_printfs to print into its local sbuf instead of writing into buf. Other ->read() implementations write file contents into buf and return their own error codes when read failures occur.

I could do if (sbuf_len(sb) == 0) { sbuf_bcpy(sb, buf, strlen(buf)) } to see if a seq_printf went through and sbuf_bcpy based on that, but I feel like the existing code is more readable.

Either way, I need the buf[0] = '\0' for strlen(). Please tell me if I am misunderstanding.

jfree: The `buf[0] = '\0'` exists for situations where nothing is written into `buf`. A good example…
rc = d->dm_fops->write(&lf, buf, len, &off); rc = d->dm_fops->read(&lf, buf, sizeof(buf), &off);
bzUnsubmitted
Not Done
Inline Actions

This doesn't work for anything but your specific case most likely. The read implementations are expecting a user address and not a kernel one returned from malloc; this makes a lot of debugfs implementations fail with -EFAULT.

bz: This doesn't work for anything but your specific case most likely. The read implementations…
else if (strlen(buf) > 0)
hselaskyUnsubmitted
Done
Inline Actions

What if the length is equal to the buffer size?

You are repeating strlen(buf), should cache value in a variable.

hselasky: What if the length is equal to the buffer size? You are repeating strlen(buf), should cache…
jfreeAuthorUnsubmitted
Done
Inline Actions

If the length is equal to the buffer size, the if statement will go through. Perhaps I am not understanding what you're suggesting?

The compiler (assuming optimizations are turned on) should optimize this out and cache the strlen() value.

jfree: If the length is equal to the buffer size, the if statement will go through. Perhaps I am not…
hselaskyUnsubmitted
Done
Inline Actions

In this error case you should call d->dm_fops->release() before returning ?

In other words,

set "rc" and goto below ?

hselasky: In this error case you should call d->dm_fops->release() before returning ? In other words…
jfreeAuthorUnsubmitted
Done
Inline Actions

Absolutely. Silly mistake on my part. Thanks for pointing that out.

jfree: Absolutely. Silly mistake on my part. Thanks for pointing that out.
rc = -ENODEV; sbuf_bcpy(sb, buf, strlen(buf));
} else if (uio->uio_rw == UIO_WRITE && d->dm_fops->write) {
sbuf_finish(sb);
rc = d->dm_fops->write(&lf, sbuf_data(sb), sbuf_len(sb), &off);
} }
if (d->dm_fops->release) if (d->dm_fops->release)
d->dm_fops->release(&vn, &lf); d->dm_fops->release(&vn, &lf);
else else
single_release(&vn, &lf); single_release(&vn, &lf);
if (rc < 0) { if (rc < 0) {
#ifdef INVARIANTS #ifdef INVARIANTS
printf("%s:%d read/write failed with %d\n", __FUNCTION__, __LINE__, rc); printf("%s:%d read/write failed with %d\n", __FUNCTION__, __LINE__, rc);
▲ Show 20 Lines • Show All 43 Lines • ▼ Show 20 Lines if (dnode->d_pfs_node == NULL) {
free(dm, M_DFSINT); free(dm, M_DFSINT);
return (NULL); return (NULL);
} }
dnode->d_pfs_node->pn_data = dm; dnode->d_pfs_node->pn_data = dm;
return (dnode); return (dnode);
} }
/*
* NOTE: The _unsafe moniker denotes that @fops are not protected against debugfs
* file removals. This behavior is not implemented in FreeBSD at this time.
markjUnsubmitted
Done
Inline Actions

What does "this behaviour" refer to exactly?

markj: What does "this behaviour" refer to exactly?
*/
struct dentry * struct dentry *
debugfs_create_file_unsafe(const char *name, umode_t mode,
struct dentry *parent, void *data,
const struct file_operations *fops)
{
return (debugfs_create_file(name, mode, parent, data, fops));
}
struct dentry *
debugfs_create_mode_unsafe(const char *name, umode_t mode,
struct dentry *parent, void *data,
const struct file_operations *fops,
const struct file_operations *fops_ro,
const struct file_operations *fops_wo)
{
umode_t read = mode & S_IRUGO;
umode_t write = mode & S_IWUGO;
if (read && !write)
return (debugfs_create_file_unsafe(name, mode, parent, data, fops_ro));
if (write && !read)
return (debugfs_create_file_unsafe(name, mode, parent, data, fops_wo));
return (debugfs_create_file_unsafe(name, mode, parent, data, fops));
}
struct dentry *
debugfs_create_dir(const char *name, struct dentry *parent) debugfs_create_dir(const char *name, struct dentry *parent)
{ {
struct dentry_meta *dm; struct dentry_meta *dm;
struct dentry *dnode; struct dentry *dnode;
struct pfs_node *pnode; struct pfs_node *pnode;
dm = malloc(sizeof(*dm), M_DFSINT, M_NOWAIT | M_ZERO); dm = malloc(sizeof(*dm), M_DFSINT, M_NOWAIT | M_ZERO);
if (dm == NULL) if (dm == NULL)
▲ Show 20 Lines • Show All 65 Lines • ▼ Show 20 Lines
{ {
if (dnode == NULL) if (dnode == NULL)
return; return;
pfs_destroy(dnode->d_pfs_node); pfs_destroy(dnode->d_pfs_node);
} }
static int static int
debugfs_bool_get(void *data, uint64_t *ullval)
{
bool *bval = data;
if (*bval)
*ullval = 1;
else
*ullval = 0;
return (0);
}
static int
debugfs_bool_set(void *data, uint64_t ullval)
{
bool *bval = data;
if (ullval)
*bval = 1;
else
*bval = 0;
return (0);
}
DEFINE_DEBUGFS_ATTRIBUTE(fops_bool, debugfs_bool_get, debugfs_bool_set, "%llu\n");
DEFINE_DEBUGFS_ATTRIBUTE(fops_bool_ro, debugfs_bool_get, NULL, "%llu\n");
DEFINE_DEBUGFS_ATTRIBUTE(fops_bool_wo, NULL, debugfs_bool_set, "%llu\n");
void
debugfs_create_bool(const char *name, umode_t mode, struct dentry *parent,
bool *value)
{
debugfs_create_mode_unsafe(name, mode, parent, value, &fops_bool,
&fops_bool_ro, &fops_bool_wo);
return;
}
static int
debugfs_ulong_get(void *data, uint64_t *value)
{
uint64_t *uldata = data;
*value = *uldata;
return (0);
}
static int
debugfs_ulong_set(void *data, uint64_t value)
{
uint64_t *uldata = data;
*uldata = value;
return (0);
}
DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong, debugfs_ulong_get, debugfs_ulong_set, "%llu\n");
DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong_ro, debugfs_ulong_get, NULL, "%llu\n");
DEFINE_DEBUGFS_ATTRIBUTE(fops_ulong_wo, NULL, debugfs_ulong_set, "%llu\n");
void
debugfs_create_ulong(const char *name, umode_t mode, struct dentry *parent,
unsigned long *value)
{
debugfs_create_mode_unsafe(name, mode, parent, value, &fops_ulong,
&fops_ulong_ro, &fops_ulong_wo);
return;
}
static int
debugfs_init(PFS_INIT_ARGS) debugfs_init(PFS_INIT_ARGS)
{ {
debugfs_root = pi->pi_root; debugfs_root = pi->pi_root;
(void)debugfs_create_symlink("kcov", NULL, "/dev/kcov"); (void)debugfs_create_symlink("kcov", NULL, "/dev/kcov");
hselaskyUnsubmitted
Done
Inline Actions

If there is no return value, you can skip the return statement. Ditto for the other cases below.

hselasky: If there is no return value, you can skip the return statement. Ditto for the other cases below.
return (0); return (0);
} }
static int static int
debugfs_uninit(PFS_INIT_ARGS) debugfs_uninit(PFS_INIT_ARGS)
{ {
return (0); return (0);
} }
#ifdef PR_ALLOW_MOUNT_LINSYSFS #ifdef PR_ALLOW_MOUNT_LINSYSFS
PSEUDOFS(debugfs, 1, PR_ALLOW_MOUNT_LINSYSFS); PSEUDOFS(debugfs, 1, PR_ALLOW_MOUNT_LINSYSFS);
#else #else
PSEUDOFS(debugfs, 1, VFCF_JAIL); PSEUDOFS(debugfs, 1, VFCF_JAIL);
#endif #endif
/* Module specifications */
static moduledata_t lindebugfs_data = { "lindebugfs", NULL, NULL };
DECLARE_MODULE(lindebugfs, lindebugfs_data, SI_SUB_EXEC, SI_ORDER_MIDDLE);
MODULE_VERSION(lindebugfs, 1);
MODULE_DEPEND(lindebugfs, linuxkpi, 1, 1, 1); MODULE_DEPEND(lindebugfs, linuxkpi, 1, 1, 1);