[bug report] smack: Implement filesystem context security hooks

[bug report] smack: Implement filesystem context security hooks

[Dan Carpenter]

Hello David Howells,

The patch 1f38a2e9dab7: "smack: Implement filesystem context security
hooks" from Nov 1, 2018, leads to the following static checker
warning:

	fs/namespace.c:1014 vfs_kern_mount()
	error: passing non negative 1 to ERR_PTR

fs/namespace.c
    1004 		return ERR_CAST(fc);
    1005 
    1006 	if (name)
    1007 		ret = vfs_parse_fs_string(fc, "source",
    1008 					  name, strlen(name));
    1009 	if (!ret)
    1010 		ret = parse_monolithic_mount_data(fc, data);
    1011 	if (!ret)
    1012 		mnt = fc_mount(fc);
    1013 	else
--> 1014 		mnt = ERR_PTR(ret);
    1015 
    1016 	put_fs_context(fc);
    1017 	return mnt;
    1018 }

The 1 comes from selinux_fs_context_parse_param() and
smack_fs_context_parse_param().  That code looks something like:

security/smack/smack_lsm.c
   707  /**
   708   * smack_fs_context_parse_param - Parse a single mount parameter
   709   * @fc: The new filesystem context being constructed.
   710   * @param: The parameter.
   711   *
   712   * Returns 0 on success or -ENOMEM on error.
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

LIES!!!!

   713   */
   714  static int smack_fs_context_parse_param(struct fs_context *fc,
   715                                          struct fs_parameter *param)
   716  {
   717          struct fs_parse_result result;
   718          int opt, rc;
   719  
   720          opt = fs_parse(fc, &smack_fs_parameters, param, &result);
   721          if (opt < 0)
   722                  return opt;
   723  
   724          rc = smack_add_opt(opt, param->string, &fc->security);
   725          if (!rc) {
   726                  param->string = NULL;
   727                  rc = 1;
                        ^^^^^^
It probably should return zero though...

   728          }
   729          return rc;
   730  }

regards,
dan carpenter

Re: [bug report] smack: Implement filesystem context security hooks

[David Howells]

Dan Carpenter <dan.carpenter@oracle.com> wrote:

>    724          rc = smack_add_opt(opt, param->string, &fc->security);
>    725          if (!rc) {
>    726                  param->string = NULL;
>    727                  rc = 1;
>                         ^^^^^^
> It probably should return zero though...

Yes it should.

David