| Message ID | 1437732285-11524-7-git-send-email-l.pawelczyk@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 24, 2015 at 12:04:40PM +0200, Lukasz Pawelczyk wrote: > Smackfs/syslog is analogous to onlycap and unconfined. When not filled > they don't do anything. In such cases onlycap and unconfined displayed > nothing when read, but syslog unconditionally displayed star. This > doesn't work well with namespaces where the star could have been > unmapped. Besides the meaning of this star was different then a star > that could be written to this file. This was misleading. > > This also brings syslog read/write functions on par with onlycap and > unconfined where it is possible to reset the value to NULL as should be > possible according to comment in smackfs.c describing smack_syslog_label > variable. > > Before that the initial state was to allow (smack_syslog_label was > NULL), but after writing star to it the current had to be labeled star > as well to have an access, even thought reading the smackfs/syslog > returned the same result in both cases. > > Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com> Acked-by: Serge Hallyn <serge.hallyn@canonical.com> > --- > security/smack/smackfs.c | 42 +++++++++++++++++++++++++++--------------- > 1 file changed, 27 insertions(+), 15 deletions(-) > > diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c > index 81a2888..89f847bba 100644 > --- a/security/smack/smackfs.c > +++ b/security/smack/smackfs.c > @@ -2362,23 +2362,20 @@ static const struct file_operations smk_change_rule_ops = { > static ssize_t smk_read_syslog(struct file *filp, char __user *buf, > size_t cn, loff_t *ppos) > { > - struct smack_known *skp; > + char *smack = ""; > ssize_t rc = -EINVAL; > int asize; > > if (*ppos != 0) > return 0; > > - if (smack_syslog_label == NULL) > - skp = &smack_known_star; > - else > - skp = smack_syslog_label; > + if (smack_syslog_label != NULL) > + smack = smack_syslog_label->smk_known; > > - asize = strlen(skp->smk_known) + 1; > + asize = strlen(smack) + 1; > > if (cn >= asize) > - rc = simple_read_from_buffer(buf, cn, ppos, skp->smk_known, > - asize); > + rc = simple_read_from_buffer(buf, cn, ppos, smack, asize); > > return rc; > } > @@ -2406,16 +2403,31 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf, > if (data == NULL) > return -ENOMEM; > > - if (copy_from_user(data, buf, count) != 0) > + if (copy_from_user(data, buf, count) != 0) { > rc = -EFAULT; > - else { > - skp = smk_import_entry(data, count); > - if (IS_ERR(skp)) > - rc = PTR_ERR(skp); > - else > - smack_syslog_label = skp; > + goto freeout; > } > > + /* > + * Clear the smack_syslog_label on invalid label errors. This means > + * that we can pass a null string to unset the syslog value. > + * > + * Importing will also reject a label beginning with '-', > + * so "-syslog" will also work. > + * > + * But do so only on invalid label, not on system errors. > + */ > + skp = smk_import_entry(data, count); > + if (PTR_ERR(skp) == -EINVAL) > + skp = NULL; > + else if (IS_ERR(skp)) { > + rc = PTR_ERR(skp); > + goto freeout; > + } > + > + smack_syslog_label = skp; > + > +freeout: > kfree(data); > return rc; > } > -- > 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" 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/smack/smackfs.c b/security/smack/smackfs.c index 81a2888..89f847bba 100644 --- a/security/smack/smackfs.c +++ b/security/smack/smackfs.c @@ -2362,23 +2362,20 @@ static const struct file_operations smk_change_rule_ops = { static ssize_t smk_read_syslog(struct file *filp, char __user *buf, size_t cn, loff_t *ppos) { - struct smack_known *skp; + char *smack = ""; ssize_t rc = -EINVAL; int asize; if (*ppos != 0) return 0; - if (smack_syslog_label == NULL) - skp = &smack_known_star; - else - skp = smack_syslog_label; + if (smack_syslog_label != NULL) + smack = smack_syslog_label->smk_known; - asize = strlen(skp->smk_known) + 1; + asize = strlen(smack) + 1; if (cn >= asize) - rc = simple_read_from_buffer(buf, cn, ppos, skp->smk_known, - asize); + rc = simple_read_from_buffer(buf, cn, ppos, smack, asize); return rc; } @@ -2406,16 +2403,31 @@ static ssize_t smk_write_syslog(struct file *file, const char __user *buf, if (data == NULL) return -ENOMEM; - if (copy_from_user(data, buf, count) != 0) + if (copy_from_user(data, buf, count) != 0) { rc = -EFAULT; - else { - skp = smk_import_entry(data, count); - if (IS_ERR(skp)) - rc = PTR_ERR(skp); - else - smack_syslog_label = skp; + goto freeout; } + /* + * Clear the smack_syslog_label on invalid label errors. This means + * that we can pass a null string to unset the syslog value. + * + * Importing will also reject a label beginning with '-', + * so "-syslog" will also work. + * + * But do so only on invalid label, not on system errors. + */ + skp = smk_import_entry(data, count); + if (PTR_ERR(skp) == -EINVAL) + skp = NULL; + else if (IS_ERR(skp)) { + rc = PTR_ERR(skp); + goto freeout; + } + + smack_syslog_label = skp; + +freeout: kfree(data); return rc; }
Smackfs/syslog is analogous to onlycap and unconfined. When not filled they don't do anything. In such cases onlycap and unconfined displayed nothing when read, but syslog unconditionally displayed star. This doesn't work well with namespaces where the star could have been unmapped. Besides the meaning of this star was different then a star that could be written to this file. This was misleading. This also brings syslog read/write functions on par with onlycap and unconfined where it is possible to reset the value to NULL as should be possible according to comment in smackfs.c describing smack_syslog_label variable. Before that the initial state was to allow (smack_syslog_label was NULL), but after writing star to it the current had to be labeled star as well to have an access, even thought reading the smackfs/syslog returned the same result in both cases. Signed-off-by: Lukasz Pawelczyk <l.pawelczyk@samsung.com> --- security/smack/smackfs.c | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-)