Message ID | 38273216-97ad-7955-941a-68485534d39f@users.sourceforge.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Tue, Apr 4, 2017 at 7:16 AM, SF Markus Elfring <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Tue, 4 Apr 2017 12:23:41 +0200 > > The error code "-ENOMEM" was also returned so far when the parameter "s" > of this function contained a null pointer. > Now I find that the code "-EINVAL" is more appropriate in this case. > > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > security/selinux/ss/sidtab.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Have you tested this to determine any impact it may have on the SELinux userspace? I would agree that EINVAL is probably more appropriate in this case, but changing this return code has very little value and may disrupt userspace if it assumes EINVAL means something else when the policy load fails. Without a demonstration that all the code paths have been tested I'm not inclined to merge this patch. > diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c > index c5f436b15d19..2eb2a54b88d2 100644 > --- a/security/selinux/ss/sidtab.c > +++ b/security/selinux/ss/sidtab.c > @@ -36,7 +36,7 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) > struct sidtab_node *prev, *cur, *newnode; > > if (!s) > - return -ENOMEM; > + return -EINVAL; > > hvalue = SIDTAB_HASH(sid); > prev = NULL; > -- > 2.12.2
> Have you tested this to determine any impact it may have on the > SELinux userspace? Not yet. > I would agree that EINVAL is probably more appropriate in this case, Thanks that a part of your view seems to fit also to mine. > but changing this return code has very little value I would appreciate if this aspect can clarified a bit more. > and may disrupt userspace if it assumes EINVAL means something else > when the policy load fails. Would you find an other error code better there? Do you care to distinguish an input validation failure in a specific function implementation from other error situations? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/security/selinux/ss/sidtab.c b/security/selinux/ss/sidtab.c index c5f436b15d19..2eb2a54b88d2 100644 --- a/security/selinux/ss/sidtab.c +++ b/security/selinux/ss/sidtab.c @@ -36,7 +36,7 @@ int sidtab_insert(struct sidtab *s, u32 sid, struct context *context) struct sidtab_node *prev, *cur, *newnode; if (!s) - return -ENOMEM; + return -EINVAL; hvalue = SIDTAB_HASH(sid); prev = NULL;