Re: 2.1.15-patch[12]

Andrew Walker (andy@lysaker.kvaerner.no)
Wed, 18 Dec 1996 08:48:00 +0100 (MET)

Ion Badulescu wrote:
>
>
> Hi,
>
> The modification to locks.c in Alan's patches breaks sendmail, and
> possibly other applications as well. The author of the patch had good
> intentions (avoid random users locking files they cannot open in writing),
> however there is too much broken software out there which assumes that
> flock will work with a r/o file descriptor.
>
> The following patch does pretty much the same thing, but in a more
> conservative way.
>
> Ionut
>
> --
> It is better to keep your mouth shut and be thought a fool,
> than to open it and remove all doubt.
>
> --- linux/fs/locks.c.old Wed Dec 4 23:16:57 1996
> +++ linux/fs/locks.c Wed Dec 4 23:14:25 1996
> @@ -242,9 +242,14 @@
> {
> struct file_lock file_lock;
> struct file *filp;
> + int err;
>
> if ((fd >= NR_OPEN) || !(filp = current->files->fd[fd]))
> return (-EBADF);
> +
> + err = permission(filp->f_inode, S_IWOTH);
> + if (err)
> + return err;
>
> if (!flock_make_lock(filp, &file_lock, cmd))
> return (-EINVAL);
>

Please don't add permission checking for flock(). That is not correct
BSD practice (no matter what anyone says). I've studied FreeBSD 2.1.6
and BSD 4.4-lite in minute detail to confirm this, and anyway these
permission checks buy us little or nothing if you think the whole thing
through.

The only reason certain os'es have permission checking on flock() is
because their flock() emulation is built on top of fcntl() (which does
do permission checking). I've been discussing this very subject with
Linus and Andrew Morgan this week (I guess we should have involved Alan Cox
as well). We agreed not to add permission checks for flock(), but I do
have some patches pending that address some unfortunate "denial of service"
problems in the way flock() and fcntl() interact on Linux.

Expect to see the patches in 2.1.16 or 2.1.17.

The patches *do not*, and *cannot*, address the denial of service that
currently occurs if a user places a shared flock() lock on the "wtmp"
file. Currently "login" tries to get an exclusive flock() on "wtmp"
and will block indefinitely in this case. The solution is to change
login - e.g. let login open "wtmp" with O_RDWR|O_APPEND and not try to
lock the file exclusively.

-Andy (primary locks.c maintainer)

-- 
Andy Walker                              Kvaerner Engineering a.s.
Andrew.Walker@lysaker.kvaerner.no        P.O. Box 222, N-1324 Lysaker, Norway


   ......if the answer isn't violence, neither is your silence......


(pwei barmy army - oslo "filial")