* [PATCH 1/1] fs: strncmp() for user space buffers
@ 2016-02-28 22:50 Amber Thrall
2016-02-28 23:03 ` Al Viro
0 siblings, 1 reply; 6+ messages in thread
From: Amber Thrall @ 2016-02-28 22:50 UTC (permalink / raw)
To: Alexander Viro; +Cc: linux-fsdevel, Amber Thrall
The simple_strncmp_to_buffer() function provides an easier method for
developers to compare a kernel space buffer against user space data. This
process is done in a few drivers and may be simplified to a single function.
Signed-off-by: Amber Thrall <amber@thrall.me>
---
fs/libfs.c | 33 +++++++++++++++++++++++++++++++++
include/linux/fs.h | 1 +
2 files changed, 34 insertions(+)
diff --git a/fs/libfs.c b/fs/libfs.c
index 0ca80b2..d588f2d 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -639,6 +639,39 @@ ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
EXPORT_SYMBOL(simple_write_to_buffer);
+/**
+ * simple_strncmp_to_buffer - compare data from user space to a buffer
+ * @to: the buffer to compare to
+ * @from: the user space buffer to read from
+ * @count: the maximum number of bytes to check
+ *
+ * The simple_strncmp_to_buffer() function compares a buffer and a user space
+ * buffer. This is similar to strncmp() but between kernel and user space
+ * buffers.
+ *
+ * On success, an integer less than, equal to, or greater than zero if @to (or
+ * the first @count bytes) is found, respectively, to be less than, to match, or
+ * be greater than @from. A negative value is returned on error.
+ **/
+int simple_strncmp_to_buffer(void *to, const void __user *from, size_t count)
+{
+ size_t res;
+ char *data;
+
+ data = kmalloc(count + 1, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+ res = copy_from_user(data, from, count);
+ if (res) {
+ kfree(data);
+ return -EFAULT;
+ }
+
+ res = strncmp(data, to, count);
+ kfree(data);
+ return res;
+}
+EXPORT_SYMBOL(simple_strncmp_to_buffer);
+
/**
* memory_read_from_buffer - copy data from the buffer
* @to: the kernel space buffer to read to
* @count: the maximum number of bytes to read
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a15fe2..c7eac75 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2903,6 +2903,7 @@ extern ssize_t simple_read_from_buffer(void __user *to, size_t count,
loff_t *ppos, const void *from, size_t available);
extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
const void __user *from, size_t count);
+extern int simple_strncmp_to_buffer(void *to, const void __user *from,
+ size_t count);
extern int __generic_file_fsync(struct file *, loff_t, loff_t, int);
extern int generic_file_fsync(struct file *, loff_t, loff_t, int);
--
2.7.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/1] fs: strncmp() for user space buffers
2016-02-28 22:50 [PATCH 1/1] fs: strncmp() for user space buffers Amber Thrall
@ 2016-02-28 23:03 ` Al Viro
2016-02-28 23:10 ` Al Viro
2016-02-28 23:39 ` Amber Thrall
0 siblings, 2 replies; 6+ messages in thread
From: Al Viro @ 2016-02-28 23:03 UTC (permalink / raw)
To: Amber Thrall; +Cc: linux-fsdevel
On Sun, Feb 28, 2016 at 02:50:22PM -0800, Amber Thrall wrote:
> The simple_strncmp_to_buffer() function provides an easier method for
> developers to compare a kernel space buffer against user space data. This
> process is done in a few drivers and may be simplified to a single function.
*blink*
The name is rather confusing and I would say that semantics is unexpected
as well. I would like to see proposed users of that primitive...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] fs: strncmp() for user space buffers
2016-02-28 23:03 ` Al Viro
@ 2016-02-28 23:10 ` Al Viro
2016-02-28 23:39 ` Amber Thrall
1 sibling, 0 replies; 6+ messages in thread
From: Al Viro @ 2016-02-28 23:10 UTC (permalink / raw)
To: Amber Thrall; +Cc: linux-fsdevel
On Sun, Feb 28, 2016 at 11:03:03PM +0000, Al Viro wrote:
> On Sun, Feb 28, 2016 at 02:50:22PM -0800, Amber Thrall wrote:
> > The simple_strncmp_to_buffer() function provides an easier method for
> > developers to compare a kernel space buffer against user space data. This
> > process is done in a few drivers and may be simplified to a single function.
>
> *blink*
>
> The name is rather confusing and I would say that semantics is unexpected
> as well. I would like to see proposed users of that primitive...
BTW, such calling conventions are going to breed bugs - it's "0 if equal,
something positive if greater, something negative if less, except when
returned negative happens to be -EFAULT or -ENOMEM, in which cases it's
an error". It would be _very_ easy to get wrong in callers.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] fs: strncmp() for user space buffers
2016-02-28 23:03 ` Al Viro
2016-02-28 23:10 ` Al Viro
@ 2016-02-28 23:39 ` Amber Thrall
2016-02-29 2:10 ` Al Viro
1 sibling, 1 reply; 6+ messages in thread
From: Amber Thrall @ 2016-02-28 23:39 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
On 02/28, Al Viro wrote:
> On Sun, Feb 28, 2016 at 02:50:22PM -0800, Amber Thrall wrote:
> > The simple_strncmp_to_buffer() function provides an easier method for
> > developers to compare a kernel space buffer against user space data. This
> > process is done in a few drivers and may be simplified to a single function.
>
> *blink*
>
> The name is rather confusing and I would say that semantics is unexpected
> as well. I would like to see proposed users of that primitive...
Apologies for the confusing name, struggled to find an appropriate name
while staying consistent with the naming schemes of
simple_read/write_to_buffer() functions, as it based off of them. I'd
love to hear alternative names.
I saw possible uses for this proposed function being an easy way to
interact with debugfs, via their write file operation. For
example in the function xenvif_write_io_ring() the string "kick" is
checked for against a user space buffer.
Thanks for the fast reply.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] fs: strncmp() for user space buffers
2016-02-28 23:39 ` Amber Thrall
@ 2016-02-29 2:10 ` Al Viro
2016-02-29 15:41 ` Amber Thrall
0 siblings, 1 reply; 6+ messages in thread
From: Al Viro @ 2016-02-29 2:10 UTC (permalink / raw)
To: Amber Thrall; +Cc: linux-fsdevel
On Sun, Feb 28, 2016 at 03:39:08PM -0800, Amber Thrall wrote:
> Apologies for the confusing name, struggled to find an appropriate name
> while staying consistent with the naming schemes of
> simple_read/write_to_buffer() functions, as it based off of them. I'd
> love to hear alternative names.
>
> I saw possible uses for this proposed function being an easy way to
> interact with debugfs, via their write file operation. For
> example in the function xenvif_write_io_ring() the string "kick" is
> checked for against a user space buffer.
TBH, that caller leaves an impression of rather... poor API - "any write
of no more than 32 bytes that starts with 'k' 'i' 'c' 'k' is OK (and
everything beyond first 4 characters is ignored), anything else is
rejected, in some cases with whining into syslog, in some - quietly".
I don't know if encouraging stuff like that is a good idea...
In any case, you've ended up open-coding kmemdup_user() + strncmp() + kfree();
the problem with combining those into a single helper is that calling
conventions will be very error-prone - you have zero/positive/negative for
passing strncmp() result *and* you need to report errors somehow.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] fs: strncmp() for user space buffers
2016-02-29 2:10 ` Al Viro
@ 2016-02-29 15:41 ` Amber Thrall
0 siblings, 0 replies; 6+ messages in thread
From: Amber Thrall @ 2016-02-29 15:41 UTC (permalink / raw)
To: Al Viro; +Cc: linux-fsdevel
On 02/29, Al Viro wrote:
> On Sun, Feb 28, 2016 at 03:39:08PM -0800, Amber Thrall wrote:
> > Apologies for the confusing name, struggled to find an appropriate name
> > while staying consistent with the naming schemes of
> > simple_read/write_to_buffer() functions, as it based off of them. I'd
> > love to hear alternative names.
> >
> > I saw possible uses for this proposed function being an easy way to
> > interact with debugfs, via their write file operation. For
> > example in the function xenvif_write_io_ring() the string "kick" is
> > checked for against a user space buffer.
>
> TBH, that caller leaves an impression of rather... poor API - "any write
> of no more than 32 bytes that starts with 'k' 'i' 'c' 'k' is OK (and
> everything beyond first 4 characters is ignored), anything else is
> rejected, in some cases with whining into syslog, in some - quietly".
> I don't know if encouraging stuff like that is a good idea...
>
> In any case, you've ended up open-coding kmemdup_user() + strncmp() + kfree();
> the problem with combining those into a single helper is that calling
> conventions will be very error-prone - you have zero/positive/negative for
> passing strncmp() result *and* you need to report errors somehow.
The conflicts between strncmp() and error values hadn't crossed my mind.
The function could return the value from strncmp() via pointer, but it
wouldn't match up with strncmp's formatting making the function
confusing to use.
Thanks for the input, I'm still new to working with the kernel and have
a lot to learn.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-29 15:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-28 22:50 [PATCH 1/1] fs: strncmp() for user space buffers Amber Thrall
2016-02-28 23:03 ` Al Viro
2016-02-28 23:10 ` Al Viro
2016-02-28 23:39 ` Amber Thrall
2016-02-29 2:10 ` Al Viro
2016-02-29 15:41 ` Amber Thrall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).