public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Petr Vandrovec" <VANDROVE@vc.cvut.cz>
To: Denis Vlasenko <vda@port.imtp.ilyichevsk.odessa.ua>
Cc: linux-kernel@vger.kernel.org, acme@conectiva.com.br
Subject: Re: AUDIT: copy_from_user is a deathtrap.
Date: Wed, 22 May 2002 15:40:57 +0200	[thread overview]
Message-ID: <5E8AE8C18EA@vcnet.vc.cvut.cz> (raw)

[trimmed cc a bit]

On 22 May 02 at 14:23, Denis Vlasenko wrote:
> > On 22 May 02 at 12:27, Denis Vlasenko wrote:
> > > > As Linus and others pointed out, copy_{to_from}_user has its uses and
> > > > will stay, but something like:
> > >
> > > I don't say 'kill it', I say 'rename it so that its name tells users what
> > > return value to expect'. However, one have to weigh
> >
> > Why?
> 
> Why what? Why rename copy_to_user? Because in its current form people

Why rename it. 

> misunderstand its return value and misuse it.
> We can keep unmodified version of copy_to_user for some time for
> compatibility.

Function name is not documentation. Documentation documents function
API, or, in case documentation is not available, source does it.
copy_to_user_dude_I_return_uncopied_length_on_error_not_EFAULT_parameters_are_same_as_for_memcpy() 
is unacceptable name, and anything shorter does not document things 
you must know.

> > From copyin/out descriptions sent yesterday if you want same source code
> > running on all (BSD,SVR4,OSF/1) platforms, you must do
> >
> > if (copyin()) return [-]EFAULT;
> 
> But if I am new to Linux and just want to write my first piece of kernel
> code, copyout() is even worse than copy_to_user(): 
> it too lacks info of what it can return (0/1, 0/-EFAULT, # of copied bytes,

Hey, please change sys_read(). I'm used that read() returns -1 on error,
but now in kernel it returns some strange negative value. Please fix it.

I'm not sure that I want to use kernel which contains code written
by people who do not read API documentation. I expect that everyone 
here tests every branch in code he writes at least once - and such test
would (f.e.) quickly reveal that read(fd, NULL, 1000) does not return -1 &
set errno to EFAULT, but instead returns complete success (1000) on
affected sound drivers.

No. copy_*_user interface is documented since 2.1.0, only bad old
verify_area() in older kernels returned -EFAULT on error, and I do
not believe that coders who coded for 2.0.x did not notice completely
different interface (copy_from_user instead of verify_area + memcpy_fromfs)
during last almost 6 years.
                                            Best regards,
                                                Petr Vandrovec
                                                vandrove@vc.cvut.cz
                                                

             reply	other threads:[~2002-05-22 13:42 UTC|newest]

Thread overview: 83+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-05-22 13:40 Petr Vandrovec [this message]
2002-05-22 18:58 ` AUDIT: copy_from_user is a deathtrap Denis Vlasenko
2002-05-22 14:13   ` Ruth Ivimey-Cook
  -- strict thread matches above, loose matches on Subject: below --
2002-05-22 10:08 Petr Vandrovec
2002-05-22 16:23 ` Denis Vlasenko
     [not found] <Pine.LNX.4.44.0205191951460.22433-100000@home.transmeta.com.suse.lists.linux.kernel>
     [not found] ` <E179fAd-0005vs-00@wagner.rustcorp.com.au.suse.lists.linux.kernel>
2002-05-20 10:59   ` Andi Kleen
2002-05-19  3:38 Rusty Russell
2002-05-19  5:23 ` Linus Torvalds
2002-05-17  0:00   ` Pavel Machek
2002-05-18 21:47   ` Benjamin Herrenschmidt
2002-05-19 12:22     ` Alan Cox
2002-05-19 18:29     ` Linus Torvalds
2002-05-19 19:57       ` Roman Zippel
2002-05-20  2:06       ` Rusty Russell
2002-05-20  2:54         ` Linus Torvalds
2002-05-20  4:53           ` Rusty Russell
2002-05-19 20:12             ` Arnaldo Carvalho de Melo
2002-05-20 16:00             ` Linus Torvalds
2002-05-19 11:41   ` Alan Cox
     [not found] <E178eMm-0000NO-00@wagner.rustcorp.com.au.suse.lists.linux.kernel>
     [not found] ` <Pine.LNX.4.44.0205171936220.1524-100000@home.transmeta.com.suse.lists.linux.kernel>
2002-05-18 10:16   ` Andi Kleen
2002-05-18 16:14     ` Linus Torvalds
2002-05-19  2:10       ` Rusty Russell
2002-05-19  3:01         ` Linus Torvalds
2002-05-19  3:05           ` Larry McVoy
2002-05-19  4:01             ` Rusty Russell
2002-05-19  4:02               ` Larry McVoy
2002-05-16 23:56                 ` Pavel Machek
2002-05-16 23:56                 ` Pavel Machek
2002-05-19  3:31           ` Rusty Russell
2002-05-19  3:34             ` Linus Torvalds
2002-05-16 23:53               ` Pavel Machek
2002-05-21 20:47                 ` Linus Torvalds
2002-05-21 21:17                   ` Pavel Machek
2002-05-21 21:25                     ` Linus Torvalds
2002-05-21 21:44                     ` Alan Cox
2002-05-21 21:46                       ` Andrew Morton
2002-05-21 22:04                         ` Linus Torvalds
2002-05-21 22:21                           ` Pavel Machek
2002-05-22 13:47                             ` Alan Cox
2002-05-22 14:13                               ` Pavel Machek
2002-05-22 14:54                                 ` Alan Cox
2002-05-22 14:42                                   ` Pavel Machek
2002-05-22 15:27                                     ` Alan Cox
2002-05-22 18:58                                   ` Kasper Dupont
2002-05-22 22:02                                     ` Alan Cox
2002-05-23  3:54                                   ` Rusty Russell
2002-05-23 11:15                                     ` Edgar Toernig
2002-05-22 16:09                                 ` Linus Torvalds
2002-05-22 20:28                                   ` Pavel Machek
2002-05-22  0:47                         ` Andrea Arcangeli
2002-05-22  5:01                         ` Rusty Russell
2002-05-22  6:28                         ` Rusty Russell
2002-05-22  4:57                       ` Rusty Russell
2002-05-22 13:30                         ` Alan Cox
2002-05-22 18:43                     ` Marco Colombo
2002-05-19 20:23       ` Edgar Toernig
2002-05-19 22:44         ` Alan Cox
     [not found] <mailman.1021642692.12772.linux-kernel2news@redhat.com>
2002-05-17 17:36 ` Pete Zaitcev
2002-05-18  1:05   ` Rusty Russell
2002-05-18  2:57     ` Alan Cox
2002-05-16 23:27       ` Pavel Machek
     [not found] ` <200205191212.g4JCCLY25867@Port.imtp.ilyichevsk.odessa.ua>
     [not found]   ` <20020520112232.A8983@devserv.devel.redhat.com>
2002-05-21 10:57     ` Denis Vlasenko
2002-05-21  6:21       ` Arnaldo Carvalho de Melo
2002-05-21  8:33         ` Christoph Hellwig
2002-05-21 19:02           ` Albert D. Cahalan
2002-05-22 14:27         ` Denis Vlasenko
2002-05-17  9:27 Rusty Russell
2002-05-17  9:21 ` David S. Miller
2002-05-17  9:49   ` Rusty Russell
2002-05-17  9:35     ` David S. Miller
2002-05-17 12:26       ` Rusty Russell
2002-05-17 17:42         ` Denis Vlasenko
2002-05-17 12:17     ` Alan Cox
2002-05-17 12:21       ` Rusty Russell
2002-05-17 12:58         ` Alan Cox
2002-05-17 12:58           ` Rusty Russell
2002-05-17 13:13             ` John Levon
2002-05-17 14:52             ` Alan Cox
2002-05-18  1:26               ` Rusty Russell
2002-05-17 17:58             ` Denis Vlasenko
2002-05-18  2:37     ` Linus Torvalds
2002-05-18 15:06       ` John Alvord
2002-05-17 10:20 ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5E8AE8C18EA@vcnet.vc.cvut.cz \
    --to=vandrove@vc.cvut.cz \
    --cc=acme@conectiva.com.br \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vda@port.imtp.ilyichevsk.odessa.ua \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox