Re: [bug report] selinux: kill selinux_sb_get_mnt_opts()

[Stephen Smalley <sds@tycho.nsa.gov>]

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
>