Re: [patch-2.3.29] bugfix for pipe(2) system call.

Tigran Aivazian (tigran@sco.COM)
Wed, 24 Nov 1999 14:08:33 +0000 (GMT)


having thought about it for a bit I think - perhaps it is better to run
out of resources than to introduce (on some archs) a not so trivial to
debug race condition... so perhaps it is wiser to not apply the patch
below?

What do people think?

Tigran.

On Wed, 24 Nov 1999, Tigran Aivazian wrote:

> Hi Linus and the world,
>
> Imagine a trivial program doing
>
> while (1) pipe(0);
>
> this will cause lots of =EFAULTs for a while until we start getting
> -EMFILE because of running out of descriptors. This is not nice because
> the program may want to go on and open some other files etc. The proposed
> solution is to close those two descriptors if copy_to_user() fails. I
> understand, of course, that since we no longer hold the lock some other
> thread may have reused the descriptors but such potential race condition
> is (imho) much better than to have a blatant running out of resources in
> the failing code path of sys_pipe().
>
> The copy of the patch (for all archs that are affected) is on:
>
> http://www.ocston.org/~tigran/patches/pipe-2.3.29.patch
>
> Regards,
> ------
> Tigran A. Aivazian | http://www.sco.com
> Escalations Research Group | tel: +44-(0)1923-813796
> Santa Cruz Operation Ltd | http://www.ocston.org/~tigran
>
> diff -urN -X dontdiff linux/arch/arm/kernel/sys_arm.c 2329-pipe/arch/arm/kernel/sys_arm.c
> --- linux/arch/arm/kernel/sys_arm.c Wed Nov 24 06:23:11 1999
> +++ 2329-pipe/arch/arm/kernel/sys_arm.c Wed Nov 24 14:25:53 1999
> @@ -43,8 +43,11 @@
> error = do_pipe(fd);
> unlock_kernel();
> if (!error) {
> - if (copy_to_user(fildes, fd, 2*sizeof(int)))
> + if (copy_to_user(fildes, fd, 2*sizeof(int))) {
> error = -EFAULT;
> + sys_close(fd[0]);
> + sys_close(fd[1]);
> + }
> }
> return error;
> }
> diff -urN -X dontdiff linux/arch/i386/kernel/sys_i386.c 2329-pipe/arch/i386/kernel/sys_i386.c
> --- linux/arch/i386/kernel/sys_i386.c Wed Nov 3 19:31:09 1999
> +++ 2329-pipe/arch/i386/kernel/sys_i386.c Wed Nov 24 14:24:58 1999
> @@ -35,8 +35,11 @@
> error = do_pipe(fd);
> unlock_kernel();
> if (!error) {
> - if (copy_to_user(fildes, fd, 2*sizeof(int)))
> + if (copy_to_user(fildes, fd, 2*sizeof(int))) {
> error = -EFAULT;
> + sys_close(fd[0]);
> + sys_close(fd[1]);
> + }
> }
> return error;
> }
> diff -urN -X dontdiff linux/arch/m68k/kernel/sys_m68k.c 2329-pipe/arch/m68k/kernel/sys_m68k.c
> --- linux/arch/m68k/kernel/sys_m68k.c Tue Jan 19 18:58:34 1999
> +++ 2329-pipe/arch/m68k/kernel/sys_m68k.c Wed Nov 24 14:26:19 1999
> @@ -37,8 +37,11 @@
> lock_kernel();
> error = do_pipe(fd);
> if (!error) {
> - if (copy_to_user(fildes, fd, 2*sizeof(int)))
> + if (copy_to_user(fildes, fd, 2*sizeof(int))) {
> error = -EFAULT;
> + sys_close(fd[0]);
> + sys_close(fd[1]);
> + }
> }
> unlock_kernel();
> return error;
> diff -urN -X dontdiff linux/arch/ppc/kernel/syscalls.c 2329-pipe/arch/ppc/kernel/syscalls.c
> --- linux/arch/ppc/kernel/syscalls.c Wed Sep 8 18:59:07 1999
> +++ 2329-pipe/arch/ppc/kernel/syscalls.c Wed Nov 24 14:27:06 1999
> @@ -188,8 +188,11 @@
> error = do_pipe(fd);
> unlock_kernel();
> if (!error) {
> - if (copy_to_user(fildes, fd, 2*sizeof(int)))
> + if (copy_to_user(fildes, fd, 2*sizeof(int))) {
> error = -EFAULT;
> + sys_close(fd[0]);
> + sys_close(fd[1]);
> + }
> }
> return error;
> }
> diff -urN -X dontdiff linux/arch/sh/kernel/sys_sh.c 2329-pipe/arch/sh/kernel/sys_sh.c
> --- linux/arch/sh/kernel/sys_sh.c Sat Nov 6 18:40:31 1999
> +++ 2329-pipe/arch/sh/kernel/sys_sh.c Wed Nov 24 14:27:28 1999
> @@ -37,8 +37,11 @@
> error = do_pipe(fd);
> unlock_kernel();
> if (!error) {
> - if (copy_to_user(fildes, fd, 2*sizeof(int)))
> + if (copy_to_user(fildes, fd, 2*sizeof(int))) {
> error = -EFAULT;
> + sys_close(fd[0]);
> + sys_close(fd[1]);
> + }
> }
> return error;
> }
>
>
>
>

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