Handling ioctl() argp memory

Handling ioctl() argp memory

[leslie.polzer]

[-- Attachment #1: Type: text/plain, Size: 364 bytes --]


Hello folks,

  I'm passing an array to a ioctl.
  ioctl(2) says it needs to point at memory, so I'm allocating it.
  Do I need to, or rather, am I allowed to free the memory afterwards?

  It seems I'm getting segfaults when I free the memory...

    Leslie

-- 
gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83
http://nic-nac-project.de/~skypher/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Handling ioctl() argp memory

[Markus Rechberger]

Hi Leslie,

could you show us the sourcecode?

Markus

On 3/4/07, leslie.polzer@gmx.net <leslie.polzer@gmx.net> wrote:
>
> Hello folks,
>
>   I'm passing an array to a ioctl.
>   ioctl(2) says it needs to point at memory, so I'm allocating it.
>   Do I need to, or rather, am I allowed to free the memory afterwards?
>
>   It seems I'm getting segfaults when I free the memory...
>
>     Leslie
>
> --
> gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83
> http://nic-nac-project.de/~skypher/
>


-- 
Markus Rechberger

Re: Handling ioctl() argp memory

[leslie.polzer]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

On Sun, Mar 04, 2007 at 05:52:20PM +0100, Markus Rechberger wrote:

> could you show us the sourcecode?
Take a look at the main function of the attached file.
I'm allocating memory at the start for new_map, but when I attempt
to free it at the end, libc complains.

I think now it's not related to the ioctl, since freeing 'map' works
(which is also used with ioctl), but freeing 'mapping' does not, despite
it being not passed via ioctl.

The problem has to be somewhere else, but I cannot see where...

  Leslie

-- 
gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83
http://nic-nac-project.de/~skypher/

[-- Attachment #2: joymap.c --]
[-- Type: text/plain, Size: 4731 bytes --]

/* gcc -o joymap joymap.c */

/*
 * joymap -- map Joystick/Joypad keys via Linux JSIOSBTNMAP ioctl 
 *
 * Copyright (C) 2007  Leslie P. Polzer <polzer@gnu.org>
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the
 *
 *      Free Software Foundation, Inc.,
 *      51 Franklin Street,
 *      Fifth Floor, Boston,
 *      MA  02110-1301, USA.
 */

/* strsep */
#include <string.h>

#include <unistd.h>

/* ?int?_t */
#include <stdint.h>

/* perror */
#include <stdio.h>

/* exit, malloc */
#include <stdlib.h>

/* open */
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

#include <sys/ioctl.h>

#include <linux/input.h>
#include <linux/joystick.h>


#undef NDEBUG

static int get_buttons(int fd)
{
    int i;
    uint8_t nb;

    if (ioctl(fd, JSIOCGBUTTONS, &nb) == -1)
    {
        perror("JSIOCGBTNMAP");
        return -1;
    }

    return nb;
}


static void print_map(int fd)
{
#ifndef NDEBUG
    uint16_t* map = malloc(KEY_MAX - BTN_MISC + 1);
    int i;

    if (ioctl(fd, JSIOCGBTNMAP, map) == -1)
        perror("JSIOCGBTNMAP");

    printf("Current button map:\n");
    for (i=0; i<get_buttons(fd); ++i)
        printf("\tButton %d -> %hd\n", i, map[i]);

    free(map);

    return;
#endif
}

int main(int argc, char** argv)
{
    int fd, nb, i;

    uint16_t* map = malloc(KEY_MAX - BTN_MISC + 1);
    uint16_t* new_map = malloc(KEY_MAX - BTN_MISC + 1);
    int* mapping;


    if (argc < 2)
    {
        printf("Usage: %s DEVICE [MAP]\n\n"
               "where DEVICE may be /dev/input/js0 and MAP may be '2 1 3 4'\n"
               "to swap the first two buttons of a device with four buttons\n"
               "total.  If the number of buttons specified is lower than the\n"
               "device's total number of buttons, these buttons are assumed\n"
               "to stay like they are, i.e. '2 1' for a four button device\n"
               "will become '2 1 3 4'.\n\n"
               "Mappings will stay until the driver is reset (e.g. by\n"
               "reinserting the 'joydev' module.\n\n"
               "If MAP is not specified, the current map is printed.\n",
               argv[0]);
        exit(1);
    }

    fd = open(argv[1], O_RDWR);

    if (fd == -1)
    {
        perror("open");
        exit(1);
    }

#ifndef NDEBUG
    fprintf(stderr, "KEY_MAX: %d   BTN_MISC: %d\n", KEY_MAX, BTN_MISC);
#endif

    nb = get_buttons(fd);

    fprintf(stderr, "Device has %d buttons.\n", nb);

    if (argc < 3)
    {
        print_map(fd);
        return 0;
    }

    mapping = malloc(nb);

    i = 0;
    while (1)
    {
        char* next;
        next = strsep(&argv[2], " ,");

        if (next == NULL)
            break;

        if (atoi(next) > nb || atoi(next) < 1)
        {
            printf("Mapped buttons must be between 1 and %d.\n", nb);
            return 1;
        }

        if (i == nb)
        {
            printf("Warning: too many mappings specified, ignoring remaining.\n");
            break;
        }

        mapping[i++] = atoi(next) - 1;
    }

    /* fill */
#ifndef NDEBUG
    printf("i=%d\n", i);
#endif
    for (; i<nb; ++i)
        mapping[i] = i;

#ifndef NDEBUG
    for (i=0;i<nb; ++i)
        printf("\tmapping[%d] = %d\n", i, mapping[i]);
#endif

    if (ioctl(fd, JSIOCGBTNMAP, map) == -1)
        perror("JSIOCGBTNMAP");

    print_map(fd);

#ifndef NDEBUG
    printf("Calculating new map...\n");
#endif

    for (i=0; i<nb; ++i)
        new_map[i] = map[mapping[i]];

#ifndef NDEBUG
    for (i=0; i<nb; ++i)
        printf("\tnew_map[%d] = %hd\n", i, new_map[i]);
#endif

    if (ioctl(fd, JSIOCSBTNMAP, new_map) == -1)
        perror("JSIOCSBTNMAP");
    else
    {
        printf("New map:");
        for (i=0;i<nb;++i)
            printf(" %d", new_map[i]);
        printf("\n");
    }

    print_map(fd);

    goto EXIT;


    printf("test mode, signal to exit.\n");
    printf("waiting for events...\n");
    struct js_event e;

    while (1)
    {
        read (fd, &e, sizeof(struct js_event));
        printf("\tgot one: %d=%d\n", e.number, e.value);
    }


EXIT:
    free(map);
    //free(new_map);
    //free(mapping);

    close(fd);

    return 0;

}

Re: Handling ioctl() argp memory

[Markus Rechberger]

Did you run over it with valgrind already?


On 3/4/07, leslie.polzer@gmx.net <leslie.polzer@gmx.net> wrote:
> On Sun, Mar 04, 2007 at 05:52:20PM +0100, Markus Rechberger wrote:
>
> > could you show us the sourcecode?
> Take a look at the main function of the attached file.
> I'm allocating memory at the start for new_map, but when I attempt
> to free it at the end, libc complains.
>
> I think now it's not related to the ioctl, since freeing 'map' works
> (which is also used with ioctl), but freeing 'mapping' does not, despite
> it being not passed via ioctl.
>
> The problem has to be somewhere else, but I cannot see where...
>
>   Leslie
>
> --
> gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83
> http://nic-nac-project.de/~skypher/
>


-- 
Markus Rechberger

Re: Handling ioctl() argp memory

[Glynn Clements]

leslie.polzer@gmx.net wrote:

>   I'm passing an array to a ioctl.
>   ioctl(2) says it needs to point at memory, so I'm allocating it.
>   Do I need to, or rather, am I allowed to free the memory afterwards?

Yes. The kernel never retains pointers to user-space memory; it either
copies the data, or finishes using it before the system call returns.

>   It seems I'm getting segfaults when I free the memory...

You're probably overwriting the beginning of the allocated block.

-- 
Glynn Clements <glynn@gclements.plus.com>

Re: Handling ioctl() argp memory

[leslie.polzer]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Mon, Mar 05, 2007 at 01:54:37AM +0000, Glynn Clements wrote:

> You're probably overwriting the beginning of the allocated block.
What do you mean by that?  I can surely write to allocated memory, at
the start, end and any point between...

Besides, for the record: a short Valgrind run (thanks for the reminder!)
showed me that I calculated the space wrongly, i.e. I allocated not
enough memory.

  Leslie

-- 
gpg --keyserver pgp.mit.edu --recv-keys DD4EBF83
http://nic-nac-project.de/~skypher/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Handling ioctl() argp memory

[Glynn Clements]

leslie.polzer@gmx.net wrote:

> > You're probably overwriting the beginning of the allocated block.
> 
> What do you mean by that?  I can surely write to allocated memory, at
> the start, end and any point between...

Each block returned from malloc etc has a header; the pointer which
malloc() returns points to the first byte after the header. If you
corrupt the header, you will often get a segfault when you try to
free() the block.

> Besides, for the record: a short Valgrind run (thanks for the reminder!)
> showed me that I calculated the space wrongly, i.e. I allocated not
> enough memory.

If you write beyond the end of one block, you'll often end up
corrupting the header at the beginning of the block which follows it
in memory.

Not always, as there may be some unused space after the end of the
block; it's more likely to happen with simple programs where the
blocks are likely to be adjacent in memory.

-- 
Glynn Clements <glynn@gclements.plus.com>

Re: Handling ioctl() argp memory

[Markus Rechberger]

On 3/5/07, leslie.polzer@gmx.net <leslie.polzer@gmx.net> wrote:
> On Mon, Mar 05, 2007 at 01:54:37AM +0000, Glynn Clements wrote:
>
> > You're probably overwriting the beginning of the allocated block.
> What do you mean by that?  I can surely write to allocated memory, at
> the start, end and any point between...
>
> Besides, for the record: a short Valgrind run (thanks for the reminder!)
> showed me that I calculated the space wrongly, i.e. I allocated not
> enough memory.
>

that's what I thought. I just saw that your loop over the array didn't
depend on the defined values which were used for allocating.

Markus