On 8/13/19 1:48 AM, Greg Kroah-Hartman wrote:
On Mon, Aug 12, 2019 at 12:32:49PM -0700, Mark Salyzyn wrote:
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -30,10 +30,10 @@ struct xattr_handler {
const char *prefix;
int flags; /* fs private flags */
bool (*list)(struct dentry *dentry);
- int (*get)(const struct xattr_handler *, struct dentry *dentry,
+ int (*get)(const struct xattr_handler *handler, struct dentry *dentry,
struct inode *inode, const char *name, void *buffer,
- size_t size);
- int (*set)(const struct xattr_handler *, struct dentry *dentry,
+ size_t size, int flags);
+ int (*set)(const struct xattr_handler *handler, struct dentry *dentry,
struct inode *inode, const char *name, const void *buffer,
size_t size, int flags);
Wow, 7 arguments. Isn't there some nice rule of thumb that says once
you get more then 5, a function becomes impossible to understand?
This is a method with a pot-pourri of somewhat intuitive useful, but not
always necessary, arguments, the additional argument does not complicate
the function(s) AFAIK, but maybe its usage. Most functions do not even
reference handler, the inode is typically a derivative of dentry, The
arguments most used are the name of the attribute and the buffer/size
the results are to be placed into.
The addition of flags is actually a pattern borrowed from the [.]set
method, which provides at least 32 bits of 'control' (of which we added
only one). Before, it was an anti-pattern.
Surely this could be a structure passed in here somehow, that way when
you add the 8th argument in the future, you don't have to change
everything yet again? :)
Just be happy I provided int flags, instead of bool no_security ;->
there are a few bits there that can be used in the future.
I don't have anything concrete to offer as a replacement fix for this,
but to me this just feels really wrong...
I went through 6 different alternatives (in the overlayfs security fix
patch set) until I found this one that resonated with the security and
filesystem stakeholders. The one was a direct result of trying to reduce
the security attack surface. This code was created by threading a
needle, and evolution. I am game for a 7th alternative to solve the
unionfs set of recursive calls into acquiring the extended attributes.
-- Mark
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
