linux-kernel - Re: [V9fs-developer] [PATCH] 9p: attach-per-user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 11 Sep 2007 09:31:19 -0500
From:	"Eric Van Hensbergen" <ericvh@...il.com>
To:	"Latchesar Ionkov" <lucho@...kov.net>
Cc:	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	v9fs-developer@...ts.sourceforge.net
Subject: Re: [V9fs-developer] [PATCH] 9p: attach-per-user

On 9/3/07, Latchesar Ionkov <lucho@...kov.net> wrote:
>
> This patch improves the 9P2000 support by allowing every user to attach
> separately. The patch defines three modes of access (new mount option
> 'access'):
>

nit picks:
   * you added/changed options without updated Documentation/filesystems/9p.txt
   * you changed v9fs->extended to be part of a flags structure, that
should have been
       a separate patch
   * rename of options should have been done in a separate patch

> - attach-per-user (access=user)
>  If a user tries to access a file served by v9fs for the first time, v9fs
>  sends an attach command to the server (Tattach) specifying the user. If
>  the attach succeeds, the user can access the v9fs tree.
>  As there is no uname->uid (string->integer) mapping yet, this mode works
>  only with the 9P2000.u dialect.
>
> - allow only one user to access the tree (access=<uid>)
>  Only the user with uid can access the v9fs tree. Other users that attempt
>  to access it will get EPERM error.
>
> - do all operations as a single user (access=any)
>  V9fs does a single attach and all operations are done as a single user.
>  If this mode is selected, the v9fs behavior is identical with the current
>  one.
>

Which option is default?

>
>  /**
> - * v9fs_fid_insert - add a fid to a dentry
> + * v9fs_fid_add - add a fid to a dentry
> + * @dentry: dentry that the fid is being added to
>   * @fid: fid to add
> - * @dentry: dentry that it is being added to
>   *
>   */
>
> @@ -66,52 +67,144 @@ int v9fs_fid_add(struct dentry *dentry, struct p9_fid *fid)
>  }

Even small cleanups like this should probably be confined to a
separate patch if they are unrelated.

>
> -struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
> +static struct p9_fid *v9fs_fid_find(struct dentry *dentry, u32 uid, int any)
>  {
...
>
> -struct p9_fid *v9fs_fid_clone(struct dentry *dentry)
> +struct p9_fid *v9fs_fid_lookup(struct dentry *dentry)
>  {
...
> +
> +struct p9_fid *v9fs_fid_clone(struct dentry *dentry)
> +{

The way the patch got formatted, these look like compulsive
renames..but there's an added function and then changes to the other
two.  I think it might be because of the way you ordered the
functions.  Put new functions after the old functions and maybe this
won't happen.  And clone seems to have lost his function header.  The
code is pretty inconsistent about those these days, but I'd like to do
an audit soon and make sure we have proper comment blocks where
appropriate.

scripts/checkpatch.pl reports:

ERROR: need a space before the open parenthesis '('
#244: FILE: fs/9p/fid.c:147:
+               for(ds = dentry; !IS_ROOT(ds); ds = ds->d_parent)

ERROR: need a space before the open parenthesis '('
#275: FILE: fs/9p/fid.c:178:
+       for(d = dentry, i = n; i >= 0; i--, d = d->d_parent)

Please fix up these small bits and resubmit.

                 -eric


Also, go ahead and cc: me directly on patches, for some reason this
one missed my normal filters and got lost.  If I'm directly cc:'d it
will pop to the top of my inbox.

           -eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/