selinux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] selinux: kill selinux_sb_get_mnt_opts()
@ 2019-02-05 10:20 Dan Carpenter
  2019-02-05 13:37 ` Stephen Smalley
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-02-05 10:20 UTC (permalink / raw)
  To: viro; +Cc: selinux

Hello Al Viro,

The patch e3489f8974e1: "selinux: kill selinux_sb_get_mnt_opts()"
from Dec 13, 2018, leads to the following static checker warning:

	security/selinux/hooks.c:1075 show_sid()
	error: we previously assumed 'context' could be null (see line 1070)

security/selinux/hooks.c
    1061 static int show_sid(struct seq_file *m, u32 sid)
    1062 {
    1063 	char *context = NULL;
    1064 	u32 len;
    1065 	int rc;
    1066 
    1067 	rc = security_sid_to_context(&selinux_state, sid,
    1068 					     &context, &len);
    1069 	if (!rc) {
    1070 		bool has_comma = context && strchr(context, ',');
                                         ^^^^^^^
Can context be NULL?

    1071 
    1072 		seq_putc(m, '=');
    1073 		if (has_comma)
    1074 			seq_putc(m, '\"');
--> 1075 		seq_escape(m, context, "\"\n\\");
                                      ^^^^^^^
Because if it is, then we're toasted when we pass it to seq_escape().

    1076 		if (has_comma)
    1077 			seq_putc(m, '\"');
    1078 	}
    1079 	kfree(context);
    1080 	return rc;
    1081 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] selinux: kill selinux_sb_get_mnt_opts()
  2019-02-05 10:20 [bug report] selinux: kill selinux_sb_get_mnt_opts() Dan Carpenter
@ 2019-02-05 13:37 ` Stephen Smalley
  0 siblings, 0 replies; 2+ messages in thread
From: Stephen Smalley @ 2019-02-05 13:37 UTC (permalink / raw)
  To: Dan Carpenter, viro; +Cc: selinux, Paul Moore

On 2/5/19 5:20 AM, Dan Carpenter wrote:
> Hello Al Viro,
> 
> The patch e3489f8974e1: "selinux: kill selinux_sb_get_mnt_opts()"
> from Dec 13, 2018, leads to the following static checker warning:
> 
> 	security/selinux/hooks.c:1075 show_sid()
> 	error: we previously assumed 'context' could be null (see line 1070)
> 
> security/selinux/hooks.c
>      1061 static int show_sid(struct seq_file *m, u32 sid)
>      1062 {
>      1063 	char *context = NULL;
>      1064 	u32 len;
>      1065 	int rc;
>      1066
>      1067 	rc = security_sid_to_context(&selinux_state, sid,
>      1068 					     &context, &len);
>      1069 	if (!rc) {
>      1070 		bool has_comma = context && strchr(context, ',');
>                                           ^^^^^^^
> Can context be NULL?

No, security_sid_to_context() should only return 0 if it has set context 
to a non-NULL value.

> 
>      1071
>      1072 		seq_putc(m, '=');
>      1073 		if (has_comma)
>      1074 			seq_putc(m, '\"');
> --> 1075 		seq_escape(m, context, "\"\n\\");
>                                        ^^^^^^^
> Because if it is, then we're toasted when we pass it to seq_escape().
> 
>      1076 		if (has_comma)
>      1077 			seq_putc(m, '\"');
>      1078 	}
>      1079 	kfree(context);
>      1080 	return rc;
>      1081 }
> 
> regards,
> dan carpenter
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-02-05 13:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-05 10:20 [bug report] selinux: kill selinux_sb_get_mnt_opts() Dan Carpenter
2019-02-05 13:37 ` Stephen Smalley

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).