Hi,
found something which looks for me like a kernel/glibc syscall mismatch. At
least the parameter list of "readlink" is different in the kernel compared to
glibc, POSIX and linux-man-pages. I'm not quite sure if this difference was
intended or not ...
man 3p readlink:
ssize_t readlink(const char *restrict path, char *restrict buf, size_t bufsize);
http://www.opengroup.org/onlinepubs/000095399/functions/readlink.html:
size_t readlink(const char *restrict path, char *restrict buf, size_t bufsize);
glibc (/usr/include/unistd.h):
size_t readlink (__const char *__restrict __path, char *__restrict __buf, size_t
man 2 readlink:
ssize_t readlink(const char *path, char *buf, size_t bufsiz);
^^^^^^
linux-2.6/include/linux/syscalls.h:
asmlinkage long sys_readlink(const char __user *path, char __user *buf, int
bufsiz); ^^^
All readlink prototypes, expect the one in the kernel, have an unsigned
buffer size. Even the readlink(2) man-page, which also describes an error
statement like this:
EINVAL bufsiz is not positive.
Note: the same man-page defined bufsiz as type of size_t (unsigned).
While reviewing LTP i discovered that the "readlink03" syscall test contains a
testcase to do a functional error-path test for "EINVAL bufsiz is not positive".
This testcase is using the glibc readlink() interface, which cause a unsigned
cast of the value "-1" and let the testcase fail (actually due to gcc/glibc
fortify checks and cause a __chk_fail()).
Before workarounding the testcase, or not applying -D_FORTIFY_SOURCE=2 on LTP
build, i try to understand if there is any reason for this mismatch between
kernel and glibc/POSIX. Regarding the man-page, i'm quite certain this was a
copy&paste-error by coping the prototype from the POSIX man-page.
Even sys_readlinkat(), which got introduced a long time after sys_readlink(),
got a signed buffer size. Intended?
In the rare case all this was unintended, find patches for kernel, man-pages
and LTP to change the kernel readlink syscall interface to a unsigned buffer
size.
Thoughts?
best regards,
Daniel
[Adding the man-pages historian to the CC.]
Hi Daniel,
On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub <[email protected]> wrote:
> Hi,
>
> found something which looks for me like a kernel/glibc syscall mismatch. At
> least the parameter list of "readlink" is different in the kernel compared to
> glibc, POSIX and linux-man-pages. I'm not quite sure if this difference was
> intended or not ...
>
> man 3p readlink:
> ssize_t readlink(const char *restrict path, char *restrict buf, size_t bufsize);
>
> http://www.opengroup.org/onlinepubs/000095399/functions/readlink.html:
> size_t readlink(const char *restrict path, char *restrict buf, size_t bufsize);
>
> glibc (/usr/include/unistd.h):
> size_t readlink (__const char *__restrict __path, char *__restrict __buf, size_t
>
> man 2 readlink:
> ssize_t readlink(const char *path, char *buf, size_t bufsiz);
> ^^^^^^
> linux-2.6/include/linux/syscalls.h:
> asmlinkage long sys_readlink(const char __user *path, char __user *buf, int
> bufsiz); ^^^
>
>
> All readlink prototypes, expect the one in the kernel, have an unsigned
> buffer size. Even the readlink(2) man-page, which also describes an error
> statement like this:
>
> EINVAL bufsiz is not positive.
I agree; the inconsistency is strange. Probably it was a historical accident.
> Note: the same man-page defined bufsiz as type of size_t (unsigned).
A little history, as it appears to me...
It looks like the Linux man page came from BSD. The 4.3BSD man page
documented the type as "int" (and did not document an EINVAL error for
a negative bufsize), and even today the FreeBSD (6.2) man page
documents the type as "int" (and still does not document an EINVAL
error for this case), and that is how the argument is prototyped in
FreeBSD 6.2's <unistd.h>. (I haven't tested what FreeBSD actually
does with a negative bufsize value.)
In 1993, when Linux man-pages-1.0 took the page from BSD (that page
was timestamped 19991), it looks like someone must have changed the
type of bufsize to "size_t" in the man page SYNOPSIS. Now that could
be to match Linux libc of the time, which was already using "size_t"
(even though the then current kernel used "int"), or it could have
been to match the current standards (SUSv1, which was based on the
original POSIX.1, documents the type as "size"t").
The EINVAL error was added to man-pages-1.18 in 1997 (even though, as
you note, the type was "size_t"). I suspect (this was well before I
had any association with man-pages) that was done to reflect kernel
reality (since one could bypass glibc invoke the syscall directly),
but obviously it is inconsistent with the prototype.
> While reviewing LTP i discovered that the "readlink03" syscall test contains a
> testcase to do a functional error-path test for "EINVAL bufsiz is not positive".
> This testcase is using the glibc readlink() interface, which cause a unsigned
> cast of the value "-1" and let the testcase fail (actually due to gcc/glibc
> fortify checks and cause a __chk_fail()).
>
> Before workarounding the testcase, or not applying -D_FORTIFY_SOURCE=2 on LTP
> build, i try to understand if there is any reason for this mismatch between
> kernel and glibc/POSIX. Regarding the man-page, i'm quite certain this was a
> copy&paste-error by coping the prototype from the POSIX man-page.
(See above -- the type might have been taken either from POSIX.1 or
glibc, and that might have been quite deliberate.)
> Even sys_readlinkat(), which got introduced a long time after sys_readlink(),
> got a signed buffer size. Intended?
Probably done to match sys_readlink().
> In the rare case all this was unintended, find patches for kernel, man-pages
> and LTP to change the kernel readlink syscall interface to a unsigned buffer
> size.
>
> Thoughts?
Your proposed kernel patch is an ABI change, albeit one that quite
likely would affect no applications. So it might not hurt any one.
On the other hand, is there a benefit to making the change?
Perhaps the best think to do is simply to add a note to the man page
about this inconsistency. (It's not sufficient to just remove the
EINVAL error as you propose in one of your patches, since that can
still occur when bypassing glibc.)
Perhaps others have some thoughts?
Cheers,
Michael
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html
On Fri, Oct 24, 2008 at 05:53:25PM -0500, Michael Kerrisk wrote:
> Hi Daniel,
>
> On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub <[email protected]> wrote:
>>
>> found something which looks for me like a kernel/glibc syscall mismatch.
>>
>> The standard and the manpages and glibc have
>> ssize_t readlink(..., size_t bufsize);
>>
>> But the kernel has
>>
>> linux-2.6/include/linux/syscalls.h:
>> asmlinkage long sys_readlink(..., int bufsiz);
>>
>> All readlink prototypes, expect the one in the kernel, have an unsigned
>> buffer size.
>
> I agree; the inconsistency is strange.
Hmm. I am inclined not to agree. There is no reason to expect
any particular relation between the kernel prototype of sys_foo
(if such a function exists)
and the user space prototype of the foo() C-library function.
The POSIX standard, man-pages, libc include files document the
userspace / libc interface, not the system call interface.
The system call interface is mostly undocumented.
The kernel prototype is chosen for kernel-internal or historical reasons.
Andries
sys_ppc32.c:
/* Note: it is necessary to treat bufsiz as an unsigned int ... */
asmlinkage long compat_sys_readlink(..., u32 bufsiz)
Am Fri, 24 Oct 2008 17:53:25 -0500
schrieb "Michael Kerrisk" <[email protected]>:
> Hi Daniel,
>
> On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub <[email protected]>
> wrote:
> > EINVAL bufsiz is not positive.
> The EINVAL error was added to man-pages-1.18 in 1997 (even though, as
> you note, the type was "size_t"). I suspect (this was well before I
> had any association with man-pages) that was done to reflect kernel
> reality (since one could bypass glibc invoke the syscall directly),
> but obviously it is inconsistent with the prototype.
Actually, it's not inconsistent as described, though perhaps that is
unintentional. "Not positive" isn't the same as "negative", as zero
isn't positive either, and zero is certainly a possible value of an
unsigned type
On Friday 31 October 2008 16:02:48 Kai Henningsen wrote:
> Am Fri, 24 Oct 2008 17:53:25 -0500
>
> schrieb "Michael Kerrisk" <[email protected]>:
> > Hi Daniel,
> >
> > On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub <[email protected]>
> >
> > wrote:
> > > EINVAL bufsiz is not positive.
> >
> > The EINVAL error was added to man-pages-1.18 in 1997 (even though, as
> > you note, the type was "size_t"). ?I suspect (this was well before I
> > had any association with man-pages) that was done to reflect kernel
> > reality (since one could bypass glibc invoke the syscall directly),
> > but obviously it is inconsistent with the prototype.
>
> Actually, it's not inconsistent as described, though perhaps that is
> unintentional. "Not positive" isn't the same as "negative", as zero
> isn't positive either, and zero is certainly a possible value of an
> unsigned type
True.
But there is still the problem for the ltp syscall test "readlink03", when
using the glibc "readlink" interface, by calling readlink with a buffer size
of "-1".
Calling "-1" seems to be a valid code/error-path in the linux syscall
"readlink", since there is a check for less-equal zero.
But the less zero, condition can't be reached via the glibc "readlink"
interface since this would cause fortify-check to fail (when buliding with -
D_FORITFY_SOURCE=2).
To "workaround" the fortify check, by not compiling the testcase with -
D_FORTIFY_SOURCE=2, or trying to test the linux readlink interface by calling
directly syscall() in the testcase ... both suggestion are just workarounds -
no real solutions.
We could also just remove the testcase of buffer size "-1".
The problem is still, how to test the "readlink" syscall in LTP?
best regards,
Daniel
Daniel,
Daniel Gollub wrote:
> On Friday 31 October 2008 16:02:48 Kai Henningsen wrote:
>> Am Fri, 24 Oct 2008 17:53:25 -0500
>>
>> schrieb "Michael Kerrisk" <[email protected]>:
>>> Hi Daniel,
>>>
>>> On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub <[email protected]>
>>>
>>> wrote:
>>>> EINVAL bufsiz is not positive.
>>> The EINVAL error was added to man-pages-1.18 in 1997 (even though, as
>>> you note, the type was "size_t"). I suspect (this was well before I
>>> had any association with man-pages) that was done to reflect kernel
>>> reality (since one could bypass glibc invoke the syscall directly),
>>> but obviously it is inconsistent with the prototype.
>> Actually, it's not inconsistent as described, though perhaps that is
>> unintentional. "Not positive" isn't the same as "negative", as zero
>> isn't positive either, and zero is certainly a possible value of an
>> unsigned type
>
> True.
So, at this stage I don't plan to make any change to man-pages.
(Let me know if you think this is the wrong course.)
> But there is still the problem for the ltp syscall test "readlink03", when
> using the glibc "readlink" interface, by calling readlink with a buffer size
> of "-1".
>
> Calling "-1" seems to be a valid code/error-path in the linux syscall
> "readlink", since there is a check for less-equal zero.
>
> But the less zero, condition can't be reached via the glibc "readlink"
> interface since this would cause fortify-check to fail (when buliding with -
> D_FORITFY_SOURCE=2).
>
> To "workaround" the fortify check, by not compiling the testcase with -
> D_FORTIFY_SOURCE=2, or trying to test the linux readlink interface by calling
> directly syscall() in the testcase ... both suggestion are just workarounds -
> no real solutions.
>
> We could also just remove the testcase of buffer size "-1".
>
> The problem is still, how to test the "readlink" syscall in LTP?
I'd say: remove this test. And add one for bufsiz==0 if there isn't one
already.
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
git://git.kernel.org/pub/scm/docs/man-pages/man-pages.git
man-pages online: http://www.kernel.org/doc/man-pages/online_pages.html
Found a bug? http://www.kernel.org/doc/man-pages/reporting_bugs.html