[CHECKER] 5 possible mis-uses of IS_ERR

From: Dawson Engler (engler@csl.Stanford.EDU)
Date: Wed Mar 21 2001 - 22:55:55 EST


Hi All,

as most people know, there are some number of routines in the kernel
that pass back negative error codes as pointers, essentially
doing the cast:
        return (void *)-ENOMEM;

These values are supposed to be checked with the routine IS_ERR:
        static inline long IS_ERR(const void *ptr)
        {
                return (unsigned long)ptr > (unsigned long)-1000L;
        }

For example:
        msg = load_msg(msgp->mtext, msgsz);
        if(IS_ERR(msg))
                return PTR_ERR(msg);

A bad mistake is to check if such a routine failed by comparing the
result to null. Some code in 2.3.99 did this:

        /* ipc/shm.c:750 */
        if (!(shp = seg_alloc(...)))
                return -ENOMEM;
        id = shm_addid(shp);

If seg_alloc did fail, it wouldn't return 0, but instead would give you
back a bogus pointer with lots of high bits set. On some archtectures
using it would let you trash the corresponding chunk of physical memory.

Anyway, we wrote a check to see that IS_ERR functions were handled
consistently. The first pass sees which functions ever get checked using
IS_ERR. The second pass makes sure these always get checked that way.
It didn't find any bugs like the seg_alloc one, but it did find the
opposite case, where code does an IS_ERR check on a function that
actually returns NULL. (A problem, since IS_ERR(0) = 0)

However, these are in namei and reiserfs, so I'm not sure if I'm
misunderstanding some subtle trick.

Dawson
------------------------------------------------------------------------
[BUG] __getname is just kmem_cache_alloc. returns 0 on failure so seems to
      cause a segfault.

/u2/engler/mc/oses/linux/2.4.1/fs/namei.c:1930:__vfs_follow_link: NOTE:DERIVE_IS_ERR:EXAMPLE: 'kmem_cache_alloc':1930

        name = __getname();
        if (IS_ERR(name))
                goto fail_name;
        strcpy(name, nd->last.name);
        nd->last.name = name;
        return 0;

------------------------------------------------------------------------
[BUG] grab_cache page returns the result of __grab_cache_page which can
        return null. (doesn't seem to return an ERR_PTR)

/u2/engler/mc/oses/linux/2.4.1/fs/reiserfs/inode.c:480:convert_tail_for_hole: NOTE:DERIVE_IS_ERR:EXAMPLE: 'grab_cache_page':480

--------------------------------------------------------------------
[BUG] Ditto: block_prepare_write will deref page.

/u2/engler/mc/oses/linux/2.4.1/fs/reiserfs/inode.c:1490:grab_tail_page: NOTE:DERIVE_IS_ERR:EXAMPLE: 'grab_cache_page':1490

    page = grab_cache_page(p_s_inode->i_mapping, index) ;
    error = PTR_ERR(page) ;
    if (IS_ERR(page)) {
        goto out ;
    }
    /* start within the page of the last block in the file */
    start = (offset / blocksize) * blocksize ;
    
    error = block_prepare_write(page, start, offset,
                                reiserfs_get_block_create_0) ;

--------------------------------------------------------------------
[BUG] Ditto: reiserfs_prepare_write will deref page.
/u2/engler/mc/oses/linux/2.4.1/fs/reiserfs/ioctl.c:81:reiserfs_unpack: NOTE:DERIVE_IS_ERR:EXAMPLE: 'grab_cache_page':81

    page = grab_cache_page(inode->i_mapping, index) ;
    retval = PTR_ERR(page) ;
    if (IS_ERR(page)) {
        goto out ;
    }
    retval = reiserfs_prepare_write(NULL, page, write_from, blocksize) ;

--------------------------------------------------------------------
[BUG]
/u2/engler/mc/oses/linux/2.4.1/fs/buffer.c:1884:block_truncate_page: NOTE:DERIVE_IS_ERR:EXAMPLE: 'grab_cache_page':1884

        page = grab_cache_page(mapping, index);
        err = PTR_ERR(page);
        if (IS_ERR(page))
                goto out;

        if (!page->buffers)
                create_empty_buffers(page, inode->i_dev, blocksize);
------------------------------------------------------------------------

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



This archive was generated by hypermail 2b29 : Fri Mar 23 2001 - 21:00:16 EST