[alsa-devel] [PATCH v3 0/4] ALSA: seq: obsolete change of address limit

older
Re: [alsa-devel] [PATCH v8 1/5]...

Takashi Sakamoto

11 Aug 2016 11 Aug '16

10:58 a.m.

Hi,

ALSA sequencer core has two types of clients; application and kernel. For kernel clients, the core should handle data in kernel space, meanwhile this is achieved with a hack of change of address limit. This should be purged.

This patchset is a revised version of my previous one: [alsa-devel] obsolete change of address limit http://mailman.alsa-project.org/pipermail/alsa-devel/2016-August/110937.html

Changes in v2: - Use '_IOC_SIZE' macro to calculate the size of argument of ioctl command. - Keep redundant longer name. - Improve comments. - Merge some patches relevant to the same features.

Changes in v3: - Improve series of patch - Use 'dir' field of ioctl command to detect the length of argument

Regards

Takashi Sakamoto (4): ALSA: seq: add documentation for snd_seq_kernel_client_ctl ALSA: seq: add an alternative way to handle ioctl requests ALSA: seq: change ioctl command operation to get data in kernel space ALSA: seq: obsolete change of address limit

sound/core/seq/seq_clientmgr.c | 721 ++++++++++++++++++----------------------- sound/core/seq/seq_compat.c | 7 +- 2 files changed, 317 insertions(+), 411 deletions(-)

-- 2.7.4

Show replies by date

Takashi Sakamoto

11 Aug 11 Aug

10:58 a.m.

New subject: [alsa-devel] [PATCH 1/4] ALSA: seq: add documentation for snd_seq_kernel_client_ctl

This kernel API is used by kernel implementation. Currently, it's used for kernel clients of ALSA sequencer, while it can be used for application clients. The difference is just on address spaces of argument. In short, this kernel API can be available for application client with data in kernel space.

This commit adds a document about this.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index d6d9419..07d7c57 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2423,9 +2423,17 @@ int snd_seq_kernel_client_dispatch(int client, struct snd_seq_event * ev,

EXPORT_SYMBOL(snd_seq_kernel_client_dispatch);

-/* - * exported, called by kernel clients to perform same functions as with - * userland ioctl() +/** + * snd_seq_kernel_client_ctl - operate a command for a client with data in + * kernel space. + * @clientid: A numerical ID for a client. + * @cmd: An ioctl(2) command for ALSA sequencer operation. + * @arg: A pointer to data in kernel space. + * + * Against its name, both kernel/application client can be handled by this + * kernel API. Address space for 'arg' argument should be in kernel space. + * + * Return: 0 at success. Minus error code at failure. */ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) {

-- 2.7.4

Takashi Sakamoto

10:58 a.m.

New subject: [alsa-devel] [PATCH 2/4] ALSA: seq: add an alternative way to handle ioctl requests

ALSA sequencer is designed with two types of clients; application and kernel. Operations for each ioctl command should handle data in both of user space and kernel space, while current implementation just allows them to handle data in user space. Data in kernel space is handled with change of address limit of running tasks.

This commit adds a new table to map ioctl commands to corresponding functions. The functions get data in kernel space. Helper functions to operate kernel and application clients seek entries from the table. Especially, the helper function for application is responsible for coping from user space to kernel space.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 76 +++++++++++++++++++++++++++++++++++++++++- sound/core/seq/seq_compat.c | 5 +++ 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 07d7c57..2107129 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -2168,6 +2168,13 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,

/* -------------------------------------------------------- */

+static const struct ioctl_handler { + unsigned int cmd; + int (*func)(struct snd_seq_client *client, void *arg); +} ioctl_handlers[] = { + { 0, NULL }, +}; + static struct seq_ioctl_table { unsigned int cmd; int (*func)(struct snd_seq_client *client, void __user * arg); @@ -2204,6 +2211,63 @@ static struct seq_ioctl_table { { 0, NULL }, };

+static long seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct snd_seq_client *client = file->private_data; + /* To use kernel stack for ioctl data. */ + union ioctl_arg { + int pversion; + int client_id; + struct snd_seq_system_info system_info; + struct snd_seq_running_info running_info; + struct snd_seq_client_info client_info; + struct snd_seq_port_info port_info; + struct snd_seq_port_subscribe port_subscribe; + struct snd_seq_queue_info queue_info; + struct snd_seq_queue_status queue_status; + struct snd_seq_queue_tempo tempo; + struct snd_seq_queue_timer queue_timer; + struct snd_seq_queue_client queue_client; + struct snd_seq_client_pool client_pool; + struct snd_seq_remove_events remove_events; + struct snd_seq_query_subs query_subs; + } buf = {0}; + const struct ioctl_handler *handler; + unsigned long size; + int err; + + if (snd_BUG_ON(!client)) + return -ENXIO; + + for (handler = ioctl_handlers; handler->cmd > 0; ++handler) { + if (handler->cmd == cmd) + break; + } + if (handler->cmd == 0) + return -ENOTTY; + /* + * All of ioctl commands for ALSA sequencer get an argument of size + * within 13 bits. We can safely pick up the size from the command. + */ + size = _IOC_SIZE(handler->cmd); + if (_IOC_DIR(handler->cmd) == IOC_IN) { + if (copy_from_user(&buf, (const void __user *)arg, size)) + return -EFAULT; + } + + err = handler->func(client, &buf); + if (err >= 0) { + /* Some commands includes a bug in 'dir' field. */ + if (handler->cmd == SNDRV_SEQ_IOCTL_SET_QUEUE_CLIENT || + handler->cmd == SNDRV_SEQ_IOCTL_SET_CLIENT_POOL || + _IOC_DIR(handler->cmd) == IOC_OUT) + if (copy_to_user((void __user *)arg, &buf, size)) + return -EFAULT; + } + + return err; +} + static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, void __user *arg) { @@ -2234,9 +2298,12 @@ static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg { struct snd_seq_client *client = file->private_data;

+ if (seq_ioctl(file, cmd, arg) >= 0) + return 0; + if (snd_BUG_ON(!client)) return -ENXIO; - + return snd_seq_do_ioctl(client, cmd, (void __user *) arg); }

@@ -2437,6 +2504,7 @@ EXPORT_SYMBOL(snd_seq_kernel_client_dispatch); */ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) { + const struct ioctl_handler *handler; struct snd_seq_client *client; mm_segment_t fs; int result; @@ -2444,6 +2512,12 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) client = clientptr(clientid); if (client == NULL) return -ENXIO; + + for (handler = ioctl_handlers; handler->cmd > 0; ++handler) { + if (handler->cmd == cmd) + return handler->func(client, arg); + } + fs = snd_enter_user(); result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg); snd_leave_user(fs); diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 6517590..4cfc505 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -59,6 +59,9 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned goto error; data->kernel = NULL;

+ if (snd_seq_kernel_client_ctl(client->number, cmd, &data) >= 0) + return 0; + fs = snd_enter_user(); err = snd_seq_do_ioctl(client, cmd, data); snd_leave_user(fs); @@ -123,6 +126,8 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION: case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT: case SNDRV_SEQ_IOCTL_RUNNING_MODE: + if (seq_ioctl(file, cmd, arg) >= 0) + return 0; return snd_seq_do_ioctl(client, cmd, argp); case SNDRV_SEQ_IOCTL_CREATE_PORT32: return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp);

-- 2.7.4

Clemens Ladisch

11:08 a.m.

New subject: [alsa-devel] [PATCH 2/4] ALSA: seq: add an alternative way to handle ioctl requests

Takashi Sakamoto wrote:

...

if (_IOC_DIR(handler->cmd) == IOC_IN) {

...
    _IOC_DIR(handler->cmd) == IOC_OUT)

_IOC_DIR() returns a bit mask in which both _IOC_READ and _IOC_WRITE can be set. IOC_IN, IOC_OUT, and IOC_INOUT are shifted values. (see include/uapi/asm-generic/ioctl.h)

Regards, Clemens

Takashi Sakamoto

8:48 p.m.

New subject: [alsa-devel] [PATCH 2/4] ALSA: seq: add an alternative way to handle ioctl requests

Hi Clemens,

On Aug 11 2016 18:08, Clemens Ladisch wrote:

...

Takashi Sakamoto wrote:

...
if (_IOC_DIR(handler->cmd) == IOC_IN) {

...
    _IOC_DIR(handler->cmd) == IOC_OUT)
_IOC_DIR() returns a bit mask in which both _IOC_READ and _IOC_WRITE can be set. IOC_IN, IOC_OUT, and IOC_INOUT are shifted values. (see include/uapi/asm-generic/ioctl.h)

Thanks for the correction. Yes, it's my overlook. I should have used logical OR.

Thanks

Takashi Sakamoto

10:58 a.m.

New subject: [alsa-devel] [PATCH 3/4] ALSA: seq: change ioctl command operation to get data in kernel space

In previous commit, a new table for functions with data in kernel space is added to replace current table.

This commit changes existent functions to fit the table. These functions are added to the new table and removed from the old table.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 606 +++++++++++++++++------------------------ 1 file changed, 248 insertions(+), 358 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 2107129..019facd 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -1128,59 +1128,69 @@ static unsigned int snd_seq_poll(struct file *file, poll_table * wait)

/*-----------------------------------------------------*/

+static int snd_seq_ioctl_pversion(struct snd_seq_client *client, void *arg) +{ + int *pversion = arg; + + *pversion = SNDRV_SEQ_VERSION; + return 0; +} + +static int snd_seq_ioctl_client_id(struct snd_seq_client *client, void *arg) +{ + int *client_id = arg; + + *client_id = client->number; + return 0; +}

/* SYSTEM_INFO ioctl() */ -static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void __user *arg) +static int snd_seq_ioctl_system_info(struct snd_seq_client *client, void *arg) { - struct snd_seq_system_info info; + struct snd_seq_system_info *info = arg;

- memset(&info, 0, sizeof(info)); + memset(info, 0, sizeof(*info)); /* fill the info fields */ - info.queues = SNDRV_SEQ_MAX_QUEUES; - info.clients = SNDRV_SEQ_MAX_CLIENTS; - info.ports = SNDRV_SEQ_MAX_PORTS; - info.channels = 256; /* fixed limit */ - info.cur_clients = client_usage.cur; - info.cur_queues = snd_seq_queue_get_cur_queues(); - - if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; + info->queues = SNDRV_SEQ_MAX_QUEUES; + info->clients = SNDRV_SEQ_MAX_CLIENTS; + info->ports = SNDRV_SEQ_MAX_PORTS; + info->channels = 256; /* fixed limit */ + info->cur_clients = client_usage.cur; + info->cur_queues = snd_seq_queue_get_cur_queues(); + return 0; }

/* RUNNING_MODE ioctl() */ -static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void __user *arg) +static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void *arg) { - struct snd_seq_running_info info; + struct snd_seq_running_info *info = arg; struct snd_seq_client *cptr; int err = 0;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* requested client number */ - cptr = snd_seq_client_use_ptr(info.client); + cptr = snd_seq_client_use_ptr(info->client); if (cptr == NULL) return -ENOENT; /* don't change !!! */

#ifdef SNDRV_BIG_ENDIAN - if (! info.big_endian) { + if (!info->big_endian) { err = -EINVAL; goto __err; } #else - if (info.big_endian) { + if (info->big_endian) { err = -EINVAL; goto __err; }

#endif - if (info.cpu_mode > sizeof(long)) { + if (info->cpu_mode > sizeof(long)) { err = -EINVAL; goto __err; } - cptr->convert32 = (info.cpu_mode < sizeof(long)); + cptr->convert32 = (info->cpu_mode < sizeof(long)); __err: snd_seq_client_unlock(cptr); return err; @@ -1214,51 +1224,43 @@ static void get_client_info(struct snd_seq_client *cptr, }

static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_client_info *client_info = arg; struct snd_seq_client *cptr; - struct snd_seq_client_info client_info; - - if (copy_from_user(&client_info, arg, sizeof(client_info))) - return -EFAULT;

/* requested client number */ - cptr = snd_seq_client_use_ptr(client_info.client); + cptr = snd_seq_client_use_ptr(client_info->client); if (cptr == NULL) return -ENOENT; /* don't change !!! */

- get_client_info(cptr, &client_info); + get_client_info(cptr, client_info); snd_seq_client_unlock(cptr);

- if (copy_to_user(arg, &client_info, sizeof(client_info))) - return -EFAULT; return 0; }

/* CLIENT_INFO ioctl() */ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_client_info client_info; - - if (copy_from_user(&client_info, arg, sizeof(client_info))) - return -EFAULT; + struct snd_seq_client_info *client_info = arg;

/* it is not allowed to set the info fields for an another client */ - if (client->number != client_info.client) + if (client->number != client_info->client) return -EPERM; /* also client type must be set now */ - if (client->type != client_info.type) + if (client->type != client_info->type) return -EINVAL;

/* fill the info fields */ - if (client_info.name[0]) - strlcpy(client->name, client_info.name, sizeof(client->name)); + if (client_info->name[0]) + strlcpy(client->name, client_info->name, sizeof(client->name));

- client->filter = client_info.filter; - client->event_lost = client_info.event_lost; - memcpy(client->event_filter, client_info.event_filter, 32); + client->filter = client_info->filter; + client->event_lost = client_info->event_lost; + memcpy(client->event_filter, client_info->event_filter, 32);

return 0; } @@ -1267,30 +1269,26 @@ static int snd_seq_ioctl_set_client_info(struct snd_seq_client *client, /* * CREATE PORT ioctl() */ -static int snd_seq_ioctl_create_port(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_create_port(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client_port *port; - struct snd_seq_port_info info; struct snd_seq_port_callback *callback;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* it is not allowed to create the port for an another client */ - if (info.addr.client != client->number) + if (info->addr.client != client->number) return -EPERM;

- port = snd_seq_create_port(client, (info.flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info.addr.port : -1); + port = snd_seq_create_port(client, (info->flags & SNDRV_SEQ_PORT_FLG_GIVEN_PORT) ? info->addr.port : -1); if (port == NULL) return -ENOMEM;

- if (client->type == USER_CLIENT && info.kernel) { + if (client->type == USER_CLIENT && info->kernel) { snd_seq_delete_port(client, port->addr.port); return -EINVAL; } if (client->type == KERNEL_CLIENT) { - if ((callback = info.kernel) != NULL) { + if ((callback = info->kernel) != NULL) { if (callback->owner) port->owner = callback->owner; port->private_data = callback->private_data; @@ -1303,37 +1301,29 @@ static int snd_seq_ioctl_create_port(struct snd_seq_client *client, } }

- info.addr = port->addr; + info->addr = port->addr;

- snd_seq_set_port_info(port, &info); + snd_seq_set_port_info(port, info); snd_seq_system_client_ev_port_start(port->addr.client, port->addr.port);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }

/* * DELETE PORT ioctl() */ -static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, void *arg) { - struct snd_seq_port_info info; + struct snd_seq_port_info *info = arg; int err;

- /* set passed parameters */ - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - /* it is not allowed to remove the port for an another client */ - if (info.addr.client != client->number) + if (info->addr.client != client->number) return -EPERM;

- err = snd_seq_delete_port(client, info.addr.port); + err = snd_seq_delete_port(client, info->addr.port); if (err >= 0) - snd_seq_system_client_ev_port_exit(client->number, info.addr.port); + snd_seq_system_client_ev_port_exit(client->number, info->addr.port); return err; }

@@ -1341,32 +1331,27 @@ static int snd_seq_ioctl_delete_port(struct snd_seq_client *client, /* * GET_PORT_INFO ioctl() (on any client) */ -static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client *cptr; struct snd_seq_client_port *port; - struct snd_seq_port_info info;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - cptr = snd_seq_client_use_ptr(info.addr.client); + cptr = snd_seq_client_use_ptr(info->addr.client); if (cptr == NULL) return -ENXIO;

- port = snd_seq_port_use_ptr(cptr, info.addr.port); + port = snd_seq_port_use_ptr(cptr, info->addr.port); if (port == NULL) { snd_seq_client_unlock(cptr); return -ENOENT; /* don't change */ }

/* get port info */ - snd_seq_get_port_info(port, &info); + snd_seq_get_port_info(port, info); snd_seq_port_unlock(port); snd_seq_client_unlock(cptr);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }

@@ -1374,20 +1359,16 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, /* * SET_PORT_INFO ioctl() (only ports on this/own client) */ -static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_set_port_info(struct snd_seq_client *client, void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client_port *port; - struct snd_seq_port_info info;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - if (info.addr.client != client->number) /* only set our own ports ! */ + if (info->addr.client != client->number) /* only set our own ports ! */ return -EPERM; - port = snd_seq_port_use_ptr(client, info.addr.port); + port = snd_seq_port_use_ptr(client, info->addr.port); if (port) { - snd_seq_set_port_info(port, &info); + snd_seq_set_port_info(port, info); snd_seq_port_unlock(port); } return 0; @@ -1453,34 +1434,31 @@ int snd_seq_client_notify_subscription(int client, int port, * add to port's subscription list IOCTL interface */ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result = -EINVAL; struct snd_seq_client *receiver = NULL, *sender = NULL; struct snd_seq_client_port *sport = NULL, *dport = NULL; - struct snd_seq_port_subscribe subs; - - if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT;

- if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL) + if ((receiver = snd_seq_client_use_ptr(subs->dest.client)) == NULL) goto __end; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL) goto __end; - if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL) + if ((dport = snd_seq_port_use_ptr(receiver, subs->dest.port)) == NULL) goto __end;

- result = check_subscription_permission(client, sport, dport, &subs); + result = check_subscription_permission(client, sport, dport, subs); if (result < 0) goto __end;

/* connect them */ - result = snd_seq_port_connect(client, sender, sport, receiver, dport, &subs); + result = snd_seq_port_connect(client, sender, sport, receiver, dport, subs); if (! result) /* broadcast announce */ snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, - &subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED); + subs, SNDRV_SEQ_EVENT_PORT_SUBSCRIBED); __end: if (sport) snd_seq_port_unlock(sport); @@ -1498,33 +1476,30 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client, * remove from port's subscription list */ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result = -ENXIO; struct snd_seq_client *receiver = NULL, *sender = NULL; struct snd_seq_client_port *sport = NULL, *dport = NULL; - struct snd_seq_port_subscribe subs;

- if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT; - - if ((receiver = snd_seq_client_use_ptr(subs.dest.client)) == NULL) + if ((receiver = snd_seq_client_use_ptr(subs->dest.client)) == NULL) goto __end; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL) goto __end; - if ((dport = snd_seq_port_use_ptr(receiver, subs.dest.port)) == NULL) + if ((dport = snd_seq_port_use_ptr(receiver, subs->dest.port)) == NULL) goto __end;

- result = check_subscription_permission(client, sport, dport, &subs); + result = check_subscription_permission(client, sport, dport, subs); if (result < 0) goto __end;

- result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, &subs); + result = snd_seq_port_disconnect(client, sender, sport, receiver, dport, subs); if (! result) /* broadcast announce */ snd_seq_client_notify_subscription(SNDRV_SEQ_ADDRESS_SUBSCRIBERS, 0, - &subs, SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED); + subs, SNDRV_SEQ_EVENT_PORT_UNSUBSCRIBED); __end: if (sport) snd_seq_port_unlock(sport); @@ -1539,17 +1514,13 @@ static int snd_seq_ioctl_unsubscribe_port(struct snd_seq_client *client,

/* CREATE_QUEUE ioctl() */ -static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; int result; struct snd_seq_queue *q;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - result = snd_seq_queue_alloc(client->number, info.locked, info.flags); + result = snd_seq_queue_alloc(client->number, info->locked, info->flags); if (result < 0) return result;

@@ -1557,181 +1528,150 @@ static int snd_seq_ioctl_create_queue(struct snd_seq_client *client, if (q == NULL) return -EINVAL;

- info.queue = q->queue; - info.locked = q->locked; - info.owner = q->owner; + info->queue = q->queue; + info->locked = q->locked; + info->owner = q->owner;

/* set queue name */ - if (! info.name[0]) - snprintf(info.name, sizeof(info.name), "Queue-%d", q->queue); - strlcpy(q->name, info.name, sizeof(q->name)); + if (!info->name[0]) + snprintf(info->name, sizeof(info->name), "Queue-%d", q->queue); + strlcpy(q->name, info->name, sizeof(q->name)); queuefree(q);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }

/* DELETE_QUEUE ioctl() */ -static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_delete_queue(struct snd_seq_client *client, void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - return snd_seq_queue_delete(client->number, info.queue); + return snd_seq_queue_delete(client->number, info->queue); }

/* GET_QUEUE_INFO ioctl() */ static int snd_seq_ioctl_get_queue_info(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - q = queueptr(info.queue); + q = queueptr(info->queue); if (q == NULL) return -EINVAL;

- memset(&info, 0, sizeof(info)); - info.queue = q->queue; - info.owner = q->owner; - info.locked = q->locked; - strlcpy(info.name, q->name, sizeof(info.name)); + memset(info, 0, sizeof(*info)); + info->queue = q->queue; + info->owner = q->owner; + info->locked = q->locked; + strlcpy(info->name, q->name, sizeof(info->name)); queuefree(q);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }

/* SET_QUEUE_INFO ioctl() */ static int snd_seq_ioctl_set_queue_info(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - if (info.owner != client->number) + if (info->owner != client->number) return -EINVAL;

/* change owner/locked permission */ - if (snd_seq_queue_check_access(info.queue, client->number)) { - if (snd_seq_queue_set_owner(info.queue, client->number, info.locked) < 0) + if (snd_seq_queue_check_access(info->queue, client->number)) { + if (snd_seq_queue_set_owner(info->queue, client->number, info->locked) < 0) return -EPERM; - if (info.locked) - snd_seq_queue_use(info.queue, client->number, 1); + if (info->locked) + snd_seq_queue_use(info->queue, client->number, 1); } else { return -EPERM; }

- q = queueptr(info.queue); + q = queueptr(info->queue); if (! q) return -EINVAL; if (q->owner != client->number) { queuefree(q); return -EPERM; } - strlcpy(q->name, info.name, sizeof(q->name)); + strlcpy(q->name, info->name, sizeof(q->name)); queuefree(q);

return 0; }

/* GET_NAMED_QUEUE ioctl() */ -static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, void __user *arg) +static int snd_seq_ioctl_get_named_queue(struct snd_seq_client *client, + void *arg) { - struct snd_seq_queue_info info; + struct snd_seq_queue_info *info = arg; struct snd_seq_queue *q;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - q = snd_seq_queue_find_name(info.name); + q = snd_seq_queue_find_name(info->name); if (q == NULL) return -EINVAL; - info.queue = q->queue; - info.owner = q->owner; - info.locked = q->locked; + info->queue = q->queue; + info->owner = q->owner; + info->locked = q->locked; queuefree(q);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; - return 0; }

/* GET_QUEUE_STATUS ioctl() */ static int snd_seq_ioctl_get_queue_status(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_status status; + struct snd_seq_queue_status *status = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;

- if (copy_from_user(&status, arg, sizeof(status))) - return -EFAULT; - - queue = queueptr(status.queue); + queue = queueptr(status->queue); if (queue == NULL) return -EINVAL; - memset(&status, 0, sizeof(status)); - status.queue = queue->queue; + memset(status, 0, sizeof(*status)); + status->queue = queue->queue; tmr = queue->timer; - status.events = queue->tickq->cells + queue->timeq->cells; + status->events = queue->tickq->cells + queue->timeq->cells;

- status.time = snd_seq_timer_get_cur_time(tmr); - status.tick = snd_seq_timer_get_cur_tick(tmr); + status->time = snd_seq_timer_get_cur_time(tmr); + status->tick = snd_seq_timer_get_cur_tick(tmr);

- status.running = tmr->running; + status->running = tmr->running;

- status.flags = queue->flags; + status->flags = queue->flags; queuefree(queue);

- if (copy_to_user(arg, &status, sizeof(status))) - return -EFAULT; return 0; }

/* GET_QUEUE_TEMPO ioctl() */ static int snd_seq_ioctl_get_queue_tempo(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_tempo tempo; + struct snd_seq_queue_tempo *tempo = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;

- if (copy_from_user(&tempo, arg, sizeof(tempo))) - return -EFAULT; - - queue = queueptr(tempo.queue); + queue = queueptr(tempo->queue); if (queue == NULL) return -EINVAL; - memset(&tempo, 0, sizeof(tempo)); - tempo.queue = queue->queue; + memset(tempo, 0, sizeof(*tempo)); + tempo->queue = queue->queue; tmr = queue->timer;

- tempo.tempo = tmr->tempo; - tempo.ppq = tmr->ppq; - tempo.skew_value = tmr->skew; - tempo.skew_base = tmr->skew_base; + tempo->tempo = tmr->tempo; + tempo->ppq = tmr->ppq; + tempo->skew_value = tmr->skew; + tempo->skew_base = tmr->skew_base; queuefree(queue);

- if (copy_to_user(arg, &tempo, sizeof(tempo))) - return -EFAULT; return 0; }

@@ -1747,31 +1687,25 @@ int snd_seq_set_queue_tempo(int client, struct snd_seq_queue_tempo *tempo) EXPORT_SYMBOL(snd_seq_set_queue_tempo);

static int snd_seq_ioctl_set_queue_tempo(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_queue_tempo *tempo = arg; int result; - struct snd_seq_queue_tempo tempo; - - if (copy_from_user(&tempo, arg, sizeof(tempo))) - return -EFAULT;

- result = snd_seq_set_queue_tempo(client->number, &tempo); + result = snd_seq_set_queue_tempo(client->number, tempo); return result < 0 ? result : 0; }

/* GET_QUEUE_TIMER ioctl() */ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_timer timer; + struct snd_seq_queue_timer *timer = arg; struct snd_seq_queue *queue; struct snd_seq_timer *tmr;

- if (copy_from_user(&timer, arg, sizeof(timer))) - return -EFAULT; - - queue = queueptr(timer.queue); + queue = queueptr(timer->queue); if (queue == NULL) return -EINVAL;

@@ -1780,41 +1714,36 @@ static int snd_seq_ioctl_get_queue_timer(struct snd_seq_client *client, return -ERESTARTSYS; } tmr = queue->timer; - memset(&timer, 0, sizeof(timer)); - timer.queue = queue->queue; + memset(timer, 0, sizeof(*timer)); + timer->queue = queue->queue;

- timer.type = tmr->type; + timer->type = tmr->type; if (tmr->type == SNDRV_SEQ_TIMER_ALSA) { - timer.u.alsa.id = tmr->alsa_id; - timer.u.alsa.resolution = tmr->preferred_resolution; + timer->u.alsa.id = tmr->alsa_id; + timer->u.alsa.resolution = tmr->preferred_resolution; } mutex_unlock(&queue->timer_mutex); queuefree(queue); - if (copy_to_user(arg, &timer, sizeof(timer))) - return -EFAULT; return 0; }

/* SET_QUEUE_TIMER ioctl() */ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_queue_timer *timer = arg; int result = 0; - struct snd_seq_queue_timer timer;

- if (copy_from_user(&timer, arg, sizeof(timer))) - return -EFAULT; - - if (timer.type != SNDRV_SEQ_TIMER_ALSA) + if (timer->type != SNDRV_SEQ_TIMER_ALSA) return -EINVAL;

- if (snd_seq_queue_check_access(timer.queue, client->number)) { + if (snd_seq_queue_check_access(timer->queue, client->number)) { struct snd_seq_queue *q; struct snd_seq_timer *tmr;

- q = queueptr(timer.queue); + q = queueptr(timer->queue); if (q == NULL) return -ENXIO; if (mutex_lock_interruptible(&q->timer_mutex)) { @@ -1822,13 +1751,13 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client, return -ERESTARTSYS; } tmr = q->timer; - snd_seq_queue_timer_close(timer.queue); - tmr->type = timer.type; + snd_seq_queue_timer_close(timer->queue); + tmr->type = timer->type; if (tmr->type == SNDRV_SEQ_TIMER_ALSA) { - tmr->alsa_id = timer.u.alsa.id; - tmr->preferred_resolution = timer.u.alsa.resolution; + tmr->alsa_id = timer->u.alsa.id; + tmr->preferred_resolution = timer->u.alsa.resolution; } - result = snd_seq_queue_timer_open(timer.queue); + result = snd_seq_queue_timer_open(timer->queue); mutex_unlock(&q->timer_mutex); queuefree(q); } else { @@ -1841,38 +1770,30 @@ static int snd_seq_ioctl_set_queue_timer(struct snd_seq_client *client,

/* GET_QUEUE_CLIENT ioctl() */ static int snd_seq_ioctl_get_queue_client(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_queue_client info; + struct snd_seq_queue_client *info = arg; int used;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - used = snd_seq_queue_is_used(info.queue, client->number); + used = snd_seq_queue_is_used(info->queue, client->number); if (used < 0) return -EINVAL; - info.used = used; - info.client = client->number; + info->used = used; + info->client = client->number;

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }

/* SET_QUEUE_CLIENT ioctl() */ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_queue_client *info = arg; int err; - struct snd_seq_queue_client info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT;

- if (info.used >= 0) { - err = snd_seq_queue_use(info.queue, client->number, info.used); + if (info->used >= 0) { + err = snd_seq_queue_use(info->queue, client->number, info->used); if (err < 0) return err; } @@ -1883,78 +1804,70 @@ static int snd_seq_ioctl_set_queue_client(struct snd_seq_client *client,

/* GET_CLIENT_POOL ioctl() */ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_client_pool info; + struct snd_seq_client_pool *info = arg; struct snd_seq_client *cptr;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - cptr = snd_seq_client_use_ptr(info.client); + cptr = snd_seq_client_use_ptr(info->client); if (cptr == NULL) return -ENOENT; - memset(&info, 0, sizeof(info)); - info.client = cptr->number; - info.output_pool = cptr->pool->size; - info.output_room = cptr->pool->room; - info.output_free = info.output_pool; - info.output_free = snd_seq_unused_cells(cptr->pool); + memset(info, 0, sizeof(*info)); + info->client = cptr->number; + info->output_pool = cptr->pool->size; + info->output_room = cptr->pool->room; + info->output_free = info->output_pool; + info->output_free = snd_seq_unused_cells(cptr->pool); if (cptr->type == USER_CLIENT) { - info.input_pool = cptr->data.user.fifo_pool_size; - info.input_free = info.input_pool; + info->input_pool = cptr->data.user.fifo_pool_size; + info->input_free = info->input_pool; if (cptr->data.user.fifo) - info.input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool); + info->input_free = snd_seq_unused_cells(cptr->data.user.fifo->pool); } else { - info.input_pool = 0; - info.input_free = 0; + info->input_pool = 0; + info->input_free = 0; } snd_seq_client_unlock(cptr); - if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }

/* SET_CLIENT_POOL ioctl() */ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_client_pool info; + struct snd_seq_client_pool *info = arg; int rc;

- if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; - - if (client->number != info.client) + if (client->number != info->client) return -EINVAL; /* can't change other clients */

- if (info.output_pool >= 1 && info.output_pool <= SNDRV_SEQ_MAX_EVENTS && + if (info->output_pool >= 1 && info->output_pool <= SNDRV_SEQ_MAX_EVENTS && (! snd_seq_write_pool_allocated(client) || - info.output_pool != client->pool->size)) { + info->output_pool != client->pool->size)) { if (snd_seq_write_pool_allocated(client)) { /* remove all existing cells */ snd_seq_queue_client_leave_cells(client->number); snd_seq_pool_done(client->pool); } - client->pool->size = info.output_pool; + client->pool->size = info->output_pool; rc = snd_seq_pool_init(client->pool); if (rc < 0) return rc; } if (client->type == USER_CLIENT && client->data.user.fifo != NULL && - info.input_pool >= 1 && - info.input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS && - info.input_pool != client->data.user.fifo_pool_size) { + info->input_pool >= 1 && + info->input_pool <= SNDRV_SEQ_MAX_CLIENT_EVENTS && + info->input_pool != client->data.user.fifo_pool_size) { /* change pool size */ - rc = snd_seq_fifo_resize(client->data.user.fifo, info.input_pool); + rc = snd_seq_fifo_resize(client->data.user.fifo, info->input_pool); if (rc < 0) return rc; - client->data.user.fifo_pool_size = info.input_pool; + client->data.user.fifo_pool_size = info->input_pool; } - if (info.output_room >= 1 && - info.output_room <= client->pool->size) { - client->pool->room = info.output_room; + if (info->output_room >= 1 && + info->output_room <= client->pool->size) { + client->pool->room = info->output_room; }

return snd_seq_ioctl_get_client_pool(client, arg); @@ -1963,17 +1876,14 @@ static int snd_seq_ioctl_set_client_pool(struct snd_seq_client *client,

/* REMOVE_EVENTS ioctl() */ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, - void __user *arg) + void *arg) { - struct snd_seq_remove_events info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT; + struct snd_seq_remove_events *info = arg;

/* * Input mostly not implemented XXX. */ - if (info.remove_mode & SNDRV_SEQ_REMOVE_INPUT) { + if (info->remove_mode & SNDRV_SEQ_REMOVE_INPUT) { /* * No restrictions so for a user client we can clear * the whole fifo @@ -1982,8 +1892,8 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, snd_seq_fifo_clear(client->data.user.fifo); }

- if (info.remove_mode & SNDRV_SEQ_REMOVE_OUTPUT) - snd_seq_queue_remove_cells(client->number, &info); + if (info->remove_mode & SNDRV_SEQ_REMOVE_OUTPUT) + snd_seq_queue_remove_cells(client->number, info);

return 0; } @@ -1993,26 +1903,23 @@ static int snd_seq_ioctl_remove_events(struct snd_seq_client *client, * get subscription info */ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_port_subscribe *subs = arg; int result; struct snd_seq_client *sender = NULL; struct snd_seq_client_port *sport = NULL; - struct snd_seq_port_subscribe subs; struct snd_seq_subscribers *p;

- if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT; - result = -EINVAL; - if ((sender = snd_seq_client_use_ptr(subs.sender.client)) == NULL) + if ((sender = snd_seq_client_use_ptr(subs->sender.client)) == NULL) goto __end; - if ((sport = snd_seq_port_use_ptr(sender, subs.sender.port)) == NULL) + if ((sport = snd_seq_port_use_ptr(sender, subs->sender.port)) == NULL) goto __end; - p = snd_seq_port_get_subscription(&sport->c_src, &subs.dest); + p = snd_seq_port_get_subscription(&sport->c_src, &subs->dest); if (p) { result = 0; - subs = p->info; + *subs = p->info; } else result = -ENOENT;

@@ -2021,10 +1928,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, snd_seq_port_unlock(sport); if (sender) snd_seq_client_unlock(sender); - if (result >= 0) { - if (copy_to_user(arg, &subs, sizeof(subs))) - return -EFAULT; - } + return result; }

@@ -2032,26 +1936,22 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client, /* * get subscription info - check only its presence */ -static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, - void __user *arg) +static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg) { + struct snd_seq_query_subs *subs = arg; int result = -ENXIO; struct snd_seq_client *cptr = NULL; struct snd_seq_client_port *port = NULL; - struct snd_seq_query_subs subs; struct snd_seq_port_subs_info *group; struct list_head *p; int i;

- if (copy_from_user(&subs, arg, sizeof(subs))) - return -EFAULT; - - if ((cptr = snd_seq_client_use_ptr(subs.root.client)) == NULL) + if ((cptr = snd_seq_client_use_ptr(subs->root.client)) == NULL) goto __end; - if ((port = snd_seq_port_use_ptr(cptr, subs.root.port)) == NULL) + if ((port = snd_seq_port_use_ptr(cptr, subs->root.port)) == NULL) goto __end;

- switch (subs.type) { + switch (subs->type) { case SNDRV_SEQ_QUERY_SUBS_READ: group = &port->c_src; break; @@ -2064,22 +1964,22 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client,

down_read(&group->list_mutex); /* search for the subscriber */ - subs.num_subs = group->count; + subs->num_subs = group->count; i = 0; result = -ENOENT; list_for_each(p, &group->list_head) { - if (i++ == subs.index) { + if (i++ == subs->index) { /* found! */ struct snd_seq_subscribers *s; - if (subs.type == SNDRV_SEQ_QUERY_SUBS_READ) { + if (subs->type == SNDRV_SEQ_QUERY_SUBS_READ) { s = list_entry(p, struct snd_seq_subscribers, src_list); - subs.addr = s->info.dest; + subs->addr = s->info.dest; } else { s = list_entry(p, struct snd_seq_subscribers, dest_list); - subs.addr = s->info.sender; + subs->addr = s->info.sender; } - subs.flags = s->info.flags; - subs.queue = s->info.queue; + subs->flags = s->info.flags; + subs->queue = s->info.queue; result = 0; break; } @@ -2091,10 +1991,7 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, snd_seq_port_unlock(port); if (cptr) snd_seq_client_unlock(cptr); - if (result >= 0) { - if (copy_to_user(arg, &subs, sizeof(subs))) - return -EFAULT; - } + return result; }

@@ -2103,31 +2000,26 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, * query next client */ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_client_info *info = arg; struct snd_seq_client *cptr = NULL; - struct snd_seq_client_info info; - - if (copy_from_user(&info, arg, sizeof(info))) - return -EFAULT;

/* search for next client */ - info.client++; - if (info.client < 0) - info.client = 0; - for (; info.client < SNDRV_SEQ_MAX_CLIENTS; info.client++) { - cptr = snd_seq_client_use_ptr(info.client); + info->client++; + if (info->client < 0) + info->client = 0; + for (; info->client < SNDRV_SEQ_MAX_CLIENTS; info->client++) { + cptr = snd_seq_client_use_ptr(info->client); if (cptr) break; /* found */ } if (cptr == NULL) return -ENOENT;

- get_client_info(cptr, &info); + get_client_info(cptr, info); snd_seq_client_unlock(cptr);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }

@@ -2135,34 +2027,30 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *client, * query next port */ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client, - void __user *arg) + void *arg) { + struct snd_seq_port_info *info = arg; struct snd_seq_client *cptr; struct snd_seq_client_port *port = NULL; - struct snd_seq_port_info info;

/* search for next port */ - info.addr.port++; - port = snd_seq_port_query_nearest(cptr, &info); + info->addr.port++; + port = snd_seq_port_query_nearest(cptr, info); if (port == NULL) { snd_seq_client_unlock(cptr); return -ENOENT; }

/* get port info */ - info.addr = port->addr; - snd_seq_get_port_info(port, &info); + info->addr = port->addr; + snd_seq_get_port_info(port, info); snd_seq_port_unlock(port); snd_seq_client_unlock(cptr);

- if (copy_to_user(arg, &info, sizeof(info))) - return -EFAULT; return 0; }

@@ -2172,13 +2060,8 @@ static const struct ioctl_handler { unsigned int cmd; int (*func)(struct snd_seq_client *client, void *arg); } ioctl_handlers[] = { - { 0, NULL }, -}; - -static struct seq_ioctl_table { - unsigned int cmd; - int (*func)(struct snd_seq_client *client, void __user * arg); -} ioctl_tables[] = { + { SNDRV_SEQ_IOCTL_PVERSION, snd_seq_ioctl_pversion }, + { SNDRV_SEQ_IOCTL_CLIENT_ID, snd_seq_ioctl_client_id }, { SNDRV_SEQ_IOCTL_SYSTEM_INFO, snd_seq_ioctl_system_info }, { SNDRV_SEQ_IOCTL_RUNNING_MODE, snd_seq_ioctl_running_mode }, { SNDRV_SEQ_IOCTL_GET_CLIENT_INFO, snd_seq_ioctl_get_client_info }, @@ -2211,6 +2094,13 @@ static struct seq_ioctl_table { { 0, NULL }, };

+static struct seq_ioctl_table { + unsigned int cmd; + int (*func)(struct snd_seq_client *client, void __user * arg); +} ioctl_tables[] = { + { 0, NULL }, +}; + static long seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct snd_seq_client *client = file->private_data;

-- 2.7.4

Takashi Sakamoto

10:58 a.m.

New subject: [alsa-devel] [PATCH 4/4] ALSA: seq: obsolete change of address limit

Former commits change existent functions so that they don't handle data in kernel space. Copying from/to userspace is done outside of the functions, thus no need to change address limit of running task.

This commit obsoletes get_fs()/set_fs() and applies corresponding changes.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/seq/seq_clientmgr.c | 73 +++--------------------------------------- sound/core/seq/seq_compat.c | 12 ++----- 2 files changed, 7 insertions(+), 78 deletions(-)

diff --git a/sound/core/seq/seq_clientmgr.c b/sound/core/seq/seq_clientmgr.c index 019facd..91e51e2 100644 --- a/sound/core/seq/seq_clientmgr.c +++ b/sound/core/seq/seq_clientmgr.c @@ -87,21 +87,6 @@ static int snd_seq_deliver_single_event(struct snd_seq_client *client,

/* */ - -static inline mm_segment_t snd_enter_user(void) -{ - mm_segment_t fs = get_fs(); - set_fs(get_ds()); - return fs; -} - -static inline void snd_leave_user(mm_segment_t fs) -{ - set_fs(fs); -} - -/* - */ static inline unsigned short snd_seq_file_flags(struct file *file) { switch (file->f_mode & (FMODE_READ | FMODE_WRITE)) { @@ -2094,14 +2079,8 @@ static const struct ioctl_handler { { 0, NULL }, };

-static struct seq_ioctl_table { - unsigned int cmd; - int (*func)(struct snd_seq_client *client, void __user * arg); -} ioctl_tables[] = { - { 0, NULL }, -}; - -static long seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +static long snd_seq_ioctl(struct file *file, unsigned int cmd, + unsigned long arg) { struct snd_seq_client *client = file->private_data; /* To use kernel stack for ioctl data. */ @@ -2158,45 +2137,6 @@ static long seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) return err; }

-static int snd_seq_do_ioctl(struct snd_seq_client *client, unsigned int cmd, - void __user *arg) -{ - struct seq_ioctl_table *p; - - switch (cmd) { - case SNDRV_SEQ_IOCTL_PVERSION: - /* return sequencer version number */ - return put_user(SNDRV_SEQ_VERSION, (int __user *)arg) ? -EFAULT : 0; - case SNDRV_SEQ_IOCTL_CLIENT_ID: - /* return the id of this client */ - return put_user(client->number, (int __user *)arg) ? -EFAULT : 0; - } - - if (! arg) - return -EFAULT; - for (p = ioctl_tables; p->cmd; p++) { - if (p->cmd == cmd) - return p->func(client, arg); - } - pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n", - cmd, _IOC_TYPE(cmd), _IOC_NR(cmd)); - return -ENOTTY; -} - - -static long snd_seq_ioctl(struct file *file, unsigned int cmd, unsigned long arg) -{ - struct snd_seq_client *client = file->private_data; - - if (seq_ioctl(file, cmd, arg) >= 0) - return 0; - - if (snd_BUG_ON(!client)) - return -ENXIO; - - return snd_seq_do_ioctl(client, cmd, (void __user *) arg); -} - #ifdef CONFIG_COMPAT #include "seq_compat.c" #else @@ -2396,8 +2336,6 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) { const struct ioctl_handler *handler; struct snd_seq_client *client; - mm_segment_t fs; - int result;

client = clientptr(clientid); if (client == NULL) @@ -2408,10 +2346,9 @@ int snd_seq_kernel_client_ctl(int clientid, unsigned int cmd, void *arg) return handler->func(client, arg); }

- fs = snd_enter_user(); - result = snd_seq_do_ioctl(client, cmd, (void __force __user *)arg); - snd_leave_user(fs); - return result; + pr_debug("ALSA: seq unknown ioctl() 0x%x (type='%c', number=0x%02x)\n", + cmd, _IOC_TYPE(cmd), _IOC_NR(cmd)); + return -ENOTTY; }

EXPORT_SYMBOL(snd_seq_kernel_client_ctl); diff --git a/sound/core/seq/seq_compat.c b/sound/core/seq/seq_compat.c index 4cfc505..fce5697 100644 --- a/sound/core/seq/seq_compat.c +++ b/sound/core/seq/seq_compat.c @@ -47,7 +47,6 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned { int err = -EFAULT; struct snd_seq_port_info *data; - mm_segment_t fs;

data = kmalloc(sizeof(*data), GFP_KERNEL); if (!data) @@ -59,12 +58,7 @@ static int snd_seq_call_port_info_ioctl(struct snd_seq_client *client, unsigned goto error; data->kernel = NULL;

- if (snd_seq_kernel_client_ctl(client->number, cmd, &data) >= 0) - return 0; - - fs = snd_enter_user(); - err = snd_seq_do_ioctl(client, cmd, data); - snd_leave_user(fs); + err = snd_seq_kernel_client_ctl(client->number, cmd, &data); if (err < 0) goto error;

@@ -126,9 +120,7 @@ static long snd_seq_ioctl_compat(struct file *file, unsigned int cmd, unsigned l case SNDRV_SEQ_IOCTL_GET_SUBSCRIPTION: case SNDRV_SEQ_IOCTL_QUERY_NEXT_CLIENT: case SNDRV_SEQ_IOCTL_RUNNING_MODE: - if (seq_ioctl(file, cmd, arg) >= 0) - return 0; - return snd_seq_do_ioctl(client, cmd, argp); + return snd_seq_ioctl(file, cmd, arg); case SNDRV_SEQ_IOCTL_CREATE_PORT32: return snd_seq_call_port_info_ioctl(client, SNDRV_SEQ_IOCTL_CREATE_PORT, argp); case SNDRV_SEQ_IOCTL_DELETE_PORT32:

-- 2.7.4

Takashi Iwai

4:29 p.m.

New subject: [alsa-devel] [PATCH v3 0/4] ALSA: seq: obsolete change of address limit

On Thu, 11 Aug 2016 10:58:30 +0200, Takashi Sakamoto wrote:

...

Hi,

ALSA sequencer core has two types of clients; application and kernel. For kernel clients, the core should handle data in kernel space, meanwhile this is achieved with a hack of change of address limit. This should be purged.

This patchset is a revised version of my previous one: [alsa-devel] obsolete change of address limit http://mailman.alsa-project.org/pipermail/alsa-devel/2016-August/110937.html

Changes in v2:

Use '_IOC_SIZE' macro to calculate the size of argument of ioctl command.

Keep redundant longer name.

Improve comments.

Merge some patches relevant to the same features.

Changes in v3:

Improve series of patch

Use 'dir' field of ioctl command to detect the length of argument

The patchset looks much better now. The only issue I see is what Clemens suggested. So I'll merge v4 patchset once when you resubmit with the fix.

But for v4 set, please put "v4" in the patch subjects, not only in the cover letter. You can generate it via a proper prefix option with git send-email or git format-patch.

thanks,

Takashi

Takashi Sakamoto

8:49 p.m.

New subject: [alsa-devel] [PATCH v3 0/4] ALSA: seq: obsolete change of address limit

On Aug 11 2016 23:29, Takashi Iwai wrote:

...

The patchset looks much better now. The only issue I see is what Clemens suggested. So I'll merge v4 patchset once when you resubmit with the fix.

But for v4 set, please put "v4" in the patch subjects, not only in the cover letter. You can generate it via a proper prefix option with git send-email or git format-patch.

OK. I'll post later with these requests.

Regards

Takashi Sakamoto

3055

Age (days ago)

3055

Last active (days ago)

List overview

Download

8 comments

3 participants

tags (0)

participants (3)

Clemens Ladisch
Takashi Iwai
Takashi Sakamoto