[patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall? - Kernel

This is a discussion on [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall? - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

  1. [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

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

  2. [patch 1/3] [RFC] Change sys_readlink/sys_readlinkat buffer size parameter to size_t (POSIX mismatch)

    man-page for readlink syscalls prototype with buffer size as type of size_t:
    "ssize_t readlink(const char *path, char *buf, size_t bufsiz);"

    Kernel readlink syscall interface has signed buffer size and doesn't match with
    man-page described prototype.

    POSIX and glibc implementation of the readlink interface have an unsigned buffer size.

    This patch changes the syscall interface of sys_readlink and sys_readlinkat, by
    changing the buffer size argument type to "size_t".

    A buffer size of 0 will end in return value 0. This is _different_ behavior to
    original sys_readlink/sys_readlinkat interface. But equal/similar to POSIXs one.


    Signed-off-by: Daniel Gollub


    ---

    fs/9p/vfs_inode.c | 4 ++--
    fs/bad_inode.c | 2 +-
    fs/ecryptfs/inode.c | 2 +-
    fs/gfs2/ops_inode.c | 2 +-
    fs/hppfs/hppfs.c | 2 +-
    fs/namei.c | 8 ++++----
    fs/ocfs2/symlink.c | 2 +-
    fs/proc/base.c | 6 +++---
    fs/stat.c | 8 ++++----
    include/linux/fs.h | 8 ++++----
    include/linux/syscalls.h | 4 ++--
    11 files changed, 24 insertions(+), 24 deletions(-)

    diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
    index e83aa5e..dd70525 100644
    --- a/fs/9p/vfs_inode.c
    +++ b/fs/9p/vfs_inode.c
    @@ -898,7 +898,7 @@ ino_t v9fs_qid2ino(struct p9_qid *qid)
    *
    */

    -static int v9fs_readlink(struct dentry *dentry, char *buffer, int buflen)
    +static int v9fs_readlink(struct dentry *dentry, char *buffer, size_t buflen)
    {
    int retval;

    @@ -952,7 +952,7 @@ done:
    */

    static int v9fs_vfs_readlink(struct dentry *dentry, char __user * buffer,
    - int buflen)
    + size_t buflen)
    {
    int retval;
    int ret;
    diff --git a/fs/bad_inode.c b/fs/bad_inode.c
    index 5f1538c..5342a55 100644
    --- a/fs/bad_inode.c
    +++ b/fs/bad_inode.c
    @@ -238,7 +238,7 @@ static int bad_inode_rename (struct inode *old_dir, struct dentry *old_dentry,
    }

    static int bad_inode_readlink(struct dentry *dentry, char __user *buffer,
    - int buflen)
    + size_t buflen)
    {
    return -EIO;
    }
    diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c
    index 89209f0..378a1e2 100644
    --- a/fs/ecryptfs/inode.c
    +++ b/fs/ecryptfs/inode.c
    @@ -602,7 +602,7 @@ out_lock:
    }

    static int
    -ecryptfs_readlink(struct dentry *dentry, char __user * buf, int bufsiz)
    +ecryptfs_readlink(struct dentry *dentry, char __user * buf, size_t bufsiz)
    {
    int rc;
    struct dentry *lower_dentry;
    diff --git a/fs/gfs2/ops_inode.c b/fs/gfs2/ops_inode.c
    index 534e1e2..8680062 100644
    --- a/fs/gfs2/ops_inode.c
    +++ b/fs/gfs2/ops_inode.c
    @@ -894,7 +894,7 @@ out:
    */

    static int gfs2_readlink(struct dentry *dentry, char __user *user_buf,
    - int user_size)
    + size_t user_size)
    {
    struct gfs2_inode *ip = GFS2_I(dentry->d_inode);
    char array[GFS2_FAST_NAME_SIZE], *buf = array;
    diff --git a/fs/hppfs/hppfs.c b/fs/hppfs/hppfs.c
    index 2b3d182..488f95d 100644
    --- a/fs/hppfs/hppfs.c
    +++ b/fs/hppfs/hppfs.c
    @@ -637,7 +637,7 @@ static const struct super_operations hppfs_sbops = {
    };

    static int hppfs_readlink(struct dentry *dentry, char __user *buffer,
    - int buflen)
    + size_t buflen)
    {
    struct dentry *proc_dentry;

    diff --git a/fs/namei.c b/fs/namei.c
    index 4ea63ed..63e01e4 100644
    --- a/fs/namei.c
    +++ b/fs/namei.c
    @@ -2712,7 +2712,7 @@ asmlinkage long sys_rename(const char __user *oldname, const char __user *newnam
    return sys_renameat(AT_FDCWD, oldname, AT_FDCWD, newname);
    }

    -int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen, const char *link)
    +int vfs_readlink(struct dentry *dentry, char __user *buffer, size_t buflen, const char *link)
    {
    int len;

    @@ -2721,7 +2721,7 @@ int vfs_readlink(struct dentry *dentry, char __user *buffer, int buflen, const c
    goto out;

    len = strlen(link);
    - if (len > (unsigned) buflen)
    + if (len > buflen)
    len = buflen;
    if (copy_to_user(buffer, link, len))
    len = -EFAULT;
    @@ -2734,7 +2734,7 @@ out:
    * have ->follow_link() touching nd only in nd_set_link(). Using (or not
    * using) it for any given inode is up to filesystem.
    */
    -int generic_readlink(struct dentry *dentry, char __user *buffer, int buflen)
    +int generic_readlink(struct dentry *dentry, char __user *buffer, size_t buflen)
    {
    struct nameidata nd;
    void *cookie;
    @@ -2768,7 +2768,7 @@ static char *page_getlink(struct dentry * dentry, struct page **ppage)
    return kmap(page);
    }

    -int page_readlink(struct dentry *dentry, char __user *buffer, int buflen)
    +int page_readlink(struct dentry *dentry, char __user *buffer, size_t buflen)
    {
    struct page *page = NULL;
    char *s = page_getlink(dentry, &page);
    diff --git a/fs/ocfs2/symlink.c b/fs/ocfs2/symlink.c
    index cbd03df..3295ff7 100644
    --- a/fs/ocfs2/symlink.c
    +++ b/fs/ocfs2/symlink.c
    @@ -101,7 +101,7 @@ bail:

    static int ocfs2_readlink(struct dentry *dentry,
    char __user *buffer,
    - int buflen)
    + size_t buflen)
    {
    int ret;
    char *link;
    diff --git a/fs/proc/base.c b/fs/proc/base.c
    index b5918ae..14b90b9 100644
    --- a/fs/proc/base.c
    +++ b/fs/proc/base.c
    @@ -1333,7 +1333,7 @@ out:
    return ERR_PTR(error);
    }

    -static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
    +static int do_proc_readlink(struct path *path, char __user *buffer, size_t buflen)
    {
    char *tmp = (char*)__get_free_page(GFP_TEMPORARY);
    char *pathname;
    @@ -1357,7 +1357,7 @@ static int do_proc_readlink(struct path *path, char __user *buffer, int buflen)
    return len;
    }

    -static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int buflen)
    +static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, size_t buflen)
    {
    int error = -EACCES;
    struct inode *inode = dentry->d_inode;
    @@ -2246,7 +2246,7 @@ static const struct file_operations proc_coredump_filter_operations = {
    * /proc/self:
    */
    static int proc_self_readlink(struct dentry *dentry, char __user *buffer,
    - int buflen)
    + size_t buflen)
    {
    struct pid_namespace *ns = dentry->d_sb->s_fs_info;
    pid_t tgid = task_tgid_nr_ns(current, ns);
    diff --git a/fs/stat.c b/fs/stat.c
    index 7c46fbe..b521674 100644
    --- a/fs/stat.c
    +++ b/fs/stat.c
    @@ -292,13 +292,13 @@ asmlinkage long sys_newfstat(unsigned int fd, struct stat __user *statbuf)
    }

    asmlinkage long sys_readlinkat(int dfd, const char __user *pathname,
    - char __user *buf, int bufsiz)
    + char __user *buf, size_t bufsiz)
    {
    struct path path;
    int error;

    - if (bufsiz <= 0)
    - return -EINVAL;
    + if (unlikely(!bufsiz))
    + return 0;

    error = user_path_at(dfd, pathname, 0, &path);
    if (!error) {
    @@ -319,7 +319,7 @@ asmlinkage long sys_readlinkat(int dfd, const char __user *pathname,
    }

    asmlinkage long sys_readlink(const char __user *path, char __user *buf,
    - int bufsiz)
    + size_t bufsiz)
    {
    return sys_readlinkat(AT_FDCWD, path, buf, bufsiz);
    }
    diff --git a/include/linux/fs.h b/include/linux/fs.h
    index a6a625b..816dc29 100644
    --- a/include/linux/fs.h
    +++ b/include/linux/fs.h
    @@ -1333,7 +1333,7 @@ struct inode_operations {
    int (*mknod) (struct inode *,struct dentry *,int,dev_t);
    int (*rename) (struct inode *, struct dentry *,
    struct inode *, struct dentry *);
    - int (*readlink) (struct dentry *, char __user *,int);
    + int (*readlink) (struct dentry *, char __user *,size_t);
    void * (*follow_link) (struct dentry *, struct nameidata *);
    void (*put_link) (struct dentry *, struct nameidata *, void *);
    void (*truncate) (struct inode *);
    @@ -2029,16 +2029,16 @@ extern const struct file_operations generic_ro_fops;

    #define special_file(m) (S_ISCHR(m)||S_ISBLK(m)||S_ISFIFO(m)||S_ISSOCK(m))

    -extern int vfs_readlink(struct dentry *, char __user *, int, const char *);
    +extern int vfs_readlink(struct dentry *, char __user *, size_t, const char *);
    extern int vfs_follow_link(struct nameidata *, const char *);
    -extern int page_readlink(struct dentry *, char __user *, int);
    +extern int page_readlink(struct dentry *, char __user *, size_t);
    extern void *page_follow_link_light(struct dentry *, struct nameidata *);
    extern void page_put_link(struct dentry *, struct nameidata *, void *);
    extern int __page_symlink(struct inode *inode, const char *symname, int len,
    gfp_t gfp_mask);
    extern int page_symlink(struct inode *inode, const char *symname, int len);
    extern const struct inode_operations page_symlink_inode_operations;
    -extern int generic_readlink(struct dentry *, char __user *, int);
    +extern int generic_readlink(struct dentry *, char __user *, size_t);
    extern void generic_fillattr(struct inode *, struct kstat *);
    extern int vfs_getattr(struct vfsmount *, struct dentry *, struct kstat *);
    void inode_add_bytes(struct inode *inode, loff_t bytes);
    diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
    index d6ff145..e9582d1 100644
    --- a/include/linux/syscalls.h
    +++ b/include/linux/syscalls.h
    @@ -326,7 +326,7 @@ asmlinkage ssize_t sys_sendfile(int out_fd, int in_fd,
    asmlinkage ssize_t sys_sendfile64(int out_fd, int in_fd,
    loff_t __user *offset, size_t count);
    asmlinkage long sys_readlink(const char __user *path,
    - char __user *buf, int bufsiz);
    + char __user *buf, size_t bufsiz);
    asmlinkage long sys_creat(const char __user *pathname, int mode);
    asmlinkage long sys_open(const char __user *filename,
    int flags, int mode);
    @@ -581,7 +581,7 @@ asmlinkage long sys_newfstatat(int dfd, char __user *filename,
    asmlinkage long sys_fstatat64(int dfd, char __user *filename,
    struct stat64 __user *statbuf, int flag);
    asmlinkage long sys_readlinkat(int dfd, const char __user *path, char __user *buf,
    - int bufsiz);
    + size_t bufsiz);
    asmlinkage long sys_utimensat(int dfd, char __user *filename,
    struct timespec __user *utimes, int flags);
    asmlinkage long compat_sys_futimesat(unsigned int dfd, char __user *filename,

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

  3. Re: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

    [Adding the man-pages historian to the CC.]

    Hi Daniel,

    On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub 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/...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 . (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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

    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 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)

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

  5. Re: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

    Am Fri, 24 Oct 2008 17:53:25 -0500
    schrieb "Michael Kerrisk" :

    > Hi Daniel,
    >
    > On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub
    > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

    On Friday 31 October 2008 16:02:48 Kai Henningsen wrote:
    > Am Fri, 24 Oct 2008 17:53:25 -0500
    >
    > schrieb "Michael Kerrisk" :
    > > Hi Daniel,
    > >
    > > On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub
    > >
    > > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [patch 0/3] [RFC] kernel/glibc mismatch of "readlink" syscall?

    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" :
    >>> Hi Daniel,
    >>>
    >>> On Thu, Oct 23, 2008 at 9:50 AM, Daniel Gollub
    >>>
    >>> 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread