Page MenuHomeFreeBSD
Differential D35883 Diff 109957 sys/compat/linuxkpi/common/src/linux_simple_attr.c

Changeset View

Standalone View

View Options

sys/compat/linuxkpi/common/src/linux_simple_attr.c

  • This file was added.
/*-
* SPDX-License-Identifier: BSD-2-Clause-FreeBSD
*
* Copyright (c) 2022, Jake Freeland <jfree@freebsd.org>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
* ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
* IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
* ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
* FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
* DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
* OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
* OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
#include <sys/cdefs.h>
__FBSDID("$FreeBSD$");
#include <sys/types.h>
#include <linux/fs.h>
MALLOC_DEFINE(M_LSATTR, "simple_attr", "Linux Simple Attribute File");
struct simple_attr {
int (*get)(void *, uint64_t *);
int (*set)(void *, uint64_t);
void *data;
const char *fmt;
struct mutex mutex;
};
/*
* simple_attr_open: open and populate simple attribute data
*
* @inode: file inode
* @filp: file pointer
* @get: ->get() for reading file data
* @set: ->set() for writing file data
* @fmt: format specifier for data returned by @get
*
* Memory allocate a simple_attr and appropriately initialize its members.
* The simple_attr must be stored in filp->private_data.
* Simple attr files do not support seeking. Open the file as nonseekable.
*
* Return value: simple attribute file descriptor
*/
int
simple_attr_open(struct inode *inode, struct file *filp,
int (*get)(void *, uint64_t *), int (*set)(void *, uint64_t),
const char *fmt)
markjUnsubmitted
Done
Inline Actions

Style, continuation lines should be indented by four spaces.

markj: Style, continuation lines should be indented by four spaces.
{
struct simple_attr *sattr;
sattr = malloc(sizeof(*sattr), M_LSATTR, M_ZERO | M_NOWAIT);
if (sattr == NULL)
return (-ENOMEM);
sattr->get = get;
sattr->set = set;
sattr->data = inode->i_private;
sattr->fmt = fmt;
mutex_init(&sattr->mutex);
filp->private_data = (void *) sattr;
return (nonseekable_open(inode, filp));
}
/*
* simple_attr_release: release file's simple attribute data
*
* @inode: file inode
* @filp: file pointer
*
* Return value: 0
*/
int
simple_attr_release(struct inode *inode, struct file *filp)
{
free(filp->private_data, M_LSATTR);
return (0);
}
/*
* simple_attr_read: read simple attr data and transfer into buffer
*
* @filp: file pointer
* @buf: kernel space buffer
* @read_size: number of bytes to be transferred
* @ppos: starting pointer position for transfer
*
* The simple_attr structure is stored in filp->private_data.
* ->get() retrieves raw file data.
* The ->fmt specifier can format this data to be human readable.
* This output is then transferred into the @buf buffer.
*
* Return value:
* On success, number of bytes transferred
* On failure, negative signed ERRNO
*/
ssize_t
simple_attr_read(struct file *filp, char *buf, size_t read_size, loff_t *ppos)
{
struct simple_attr *sattr;
uint64_t data;
ssize_t ret;
char prebuf[24];
sattr = filp->private_data;
if (sattr->get == NULL)
return (-EFAULT);
mutex_lock(&sattr->mutex);
hselaskyUnsubmitted
Done
Inline Actions

off-by-one ??

hselasky: off-by-one ??
jfreeAuthorUnsubmitted
Done
Inline Actions

After some investigation, it appears that strscpy will error out during a zero byte transfer. I am going to apply your suggestion to make the error message more transparent. Thanks.

jfree: After some investigation, it appears that `strscpy` will error out during a zero byte transfer.
ret = sattr->get(sattr->data, &data);
if (ret)
goto unlock;
scnprintf(prebuf, sizeof(prebuf), sattr->fmt, data);
ret = strlen(prebuf) + 1;
if (*ppos >= ret || read_size < 1) {
ret = -EINVAL;
goto unlock;
}
read_size = min(ret - *ppos, read_size);
ret = strscpy(buf, prebuf + *ppos, read_size);
/* add 1 for null terminator */
if (ret > 0)
ret += 1;
unlock:
mutex_unlock(&sattr->mutex);
return (ret);
}
/*
* simple_attr_write: write contents of buffer into simple attribute file
*
* @filp: file pointer
* @buf: kernel space buffer
* @write_size: number bytes to be transferred
* @ppos: starting pointer position for transfer
*
* The simple_attr structure is stored in filp->private_data.
* Convert the @buf string to unsigned long long.
* ->set() writes unsigned long long data into the simple attr file.
*
* Return value:
* On success, number of bytes written to simple attr
* On failure, negative signed ERRNO
*/
ssize_t
simple_attr_write(struct file *filp, const char *buf, size_t write_size, loff_t *ppos)
{
struct simple_attr *sattr;
unsigned long long data;
size_t bufsize;
ssize_t ret;
sattr = filp->private_data;
bufsize = strlen(buf) + 1;
hselaskyUnsubmitted
Done
Inline Actions

I think this is off-by-one. Should be:

*ppos >= sizeof(data)

Do you agree?

hselasky: I think this is off-by-one. Should be: ``` *ppos >= sizeof(data) ``` Do you agree?
jfreeAuthorUnsubmitted
Done
Inline Actions

I do agree. I'm also changing sizeof(data) to strlen(buf) since this function applies the offset to buf, not data. Thanks for pointing that out.

jfree: I do agree. I'm also changing `sizeof(data)` to `strlen(buf)` since this function applies the…
if (sattr->set == NULL)
return (-EFAULT);
if (*ppos >= bufsize || write_size < 1)
return (-EINVAL);
mutex_lock(&sattr->mutex);
markjUnsubmitted
Done
Inline Actions

buf has a __user annotation, which indicates that buf is a userspace pointer. If that's actually the case (i.e., the annotation is correct), then we must not dereference the user pointer directly. Data should be copied into the kernel's address space first, then kstrtoull() can be called.

Also I don't think this call needs to happen under the mutex?

markj: `buf` has a `__user` annotation, which indicates that `buf` is a userspace pointer. If that's…
jfreeAuthorUnsubmitted
Done
Inline Actions

The buffer was meant to be a userspace pointer at one time, but lindebugfs uses sbufs to transfer data. I am under the impression that the user to kernel space conversion has already been done by pseudofs in pseudofs_vnops.c:1082. If I use LinuxKPI's copy_from_user() on the data in buf, I get errors. I will remove the misleading __user.

The mutex is necessary if multiple simple_attr_write()s are executed at the same time asynchronously. We don't want multiple sources overwriting each other.

jfree: The buffer was meant to be a userspace pointer at one time, but lindebugfs uses sbufs to…
ret = kstrtoull(buf + *ppos, 0, &data);
if (ret)
goto unlock;
ret = sattr->set(sattr->data, data);
if (ret)
goto unlock;
ret = bufsize - *ppos;
unlock:
mutex_unlock(&sattr->mutex);
return (ret);
}