Skip to content

Commit

Permalink
ALSA: seq: Avoid module auto-load handling at event delivery
Browse files Browse the repository at this point in the history 
snd_seq_client_use_ptr() is supposed to return the snd_seq_client
object for the given client ID, and it tries to handle the module
auto-loading when no matching object is found.  Although the module
handling is performed only conditionally with "!in_interrupt()", this
condition may be fragile, e.g. when the code is called from the ALSA
timer callback where the spinlock is temporarily disabled while the
irq is disabled.  Then his doesn't fit well and spews the error about
sleep from invalid context, as complained recently by syzbot.

Also, in general, handling the module-loading at each time if no
matching object is found is really an overkill.  It can be still
useful when performed at the top-level ioctl or proc reads, but it
shouldn't be done at event delivery at all.

For addressing the issues above, this patch disables the module
handling in snd_seq_client_use_ptr() in normal cases like event
deliveries, but allow only in limited and safe situations.
A new function client_load_and_use_ptr() is used for the cases where
the module loading can be done safely, instead.

Reported-by: syzbot+4cb9fad083898f54c517@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/67c272e5.050a0220.dc10f.0159.GAE@google.com
Cc: <stable@vger.kernel.org>
Link: https://patch.msgid.link/20250301114530.8975-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
  • Loading branch information
Takashi Iwai committed Mar 2, 2025
1 parent f479ecc commit c9ce148
Showing 1 changed file with 30 additions and 16 deletions.
46 changes: 30 additions & 16 deletions sound/core/seq/seq_clientmgr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static struct snd_seq_client *clientptr(int clientid)
return clienttab[clientid];
}

struct snd_seq_client *snd_seq_client_use_ptr(int clientid)
static struct snd_seq_client *client_use_ptr(int clientid, bool load_module)
{
unsigned long flags;
struct snd_seq_client *client;
Expand All @@ -126,7 +126,7 @@ struct snd_seq_client *snd_seq_client_use_ptr(int clientid)
}
spin_unlock_irqrestore(&clients_lock, flags);
#ifdef CONFIG_MODULES
if (!in_interrupt()) {
if (load_module) {
static DECLARE_BITMAP(client_requested, SNDRV_SEQ_GLOBAL_CLIENTS);
static DECLARE_BITMAP(card_requested, SNDRV_CARDS);

Expand Down Expand Up @@ -168,6 +168,20 @@ struct snd_seq_client *snd_seq_client_use_ptr(int clientid)
return client;
}

/* get snd_seq_client object for the given id quickly */
struct snd_seq_client *snd_seq_client_use_ptr(int clientid)
{
return client_use_ptr(clientid, false);
}

/* get snd_seq_client object for the given id;
* if not found, retry after loading the modules
*/
static struct snd_seq_client *client_load_and_use_ptr(int clientid)
{
return client_use_ptr(clientid, IS_ENABLED(CONFIG_MODULES));
}

/* Take refcount and perform ioctl_mutex lock on the given client;
* used only for OSS sequencer
* Unlock via snd_seq_client_ioctl_unlock() below
Expand All @@ -176,7 +190,7 @@ bool snd_seq_client_ioctl_lock(int clientid)
{
struct snd_seq_client *client;

client = snd_seq_client_use_ptr(clientid);
client = client_load_and_use_ptr(clientid);
if (!client)
return false;
mutex_lock(&client->ioctl_mutex);
Expand Down Expand Up @@ -1195,7 +1209,7 @@ static int snd_seq_ioctl_running_mode(struct snd_seq_client *client, void *arg)
int err = 0;

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

Expand Down Expand Up @@ -1257,7 +1271,7 @@ static int snd_seq_ioctl_get_client_info(struct snd_seq_client *client,
struct snd_seq_client *cptr;

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

Expand Down Expand Up @@ -1396,7 +1410,7 @@ static int snd_seq_ioctl_get_port_info(struct snd_seq_client *client, void *arg)
struct snd_seq_client *cptr;
struct snd_seq_client_port *port;

cptr = snd_seq_client_use_ptr(info->addr.client);
cptr = client_load_and_use_ptr(info->addr.client);
if (cptr == NULL)
return -ENXIO;

Expand Down Expand Up @@ -1503,10 +1517,10 @@ static int snd_seq_ioctl_subscribe_port(struct snd_seq_client *client,
struct snd_seq_client *receiver = NULL, *sender = NULL;
struct snd_seq_client_port *sport = NULL, *dport = NULL;

receiver = snd_seq_client_use_ptr(subs->dest.client);
receiver = client_load_and_use_ptr(subs->dest.client);
if (!receiver)
goto __end;
sender = snd_seq_client_use_ptr(subs->sender.client);
sender = client_load_and_use_ptr(subs->sender.client);
if (!sender)
goto __end;
sport = snd_seq_port_use_ptr(sender, subs->sender.port);
Expand Down Expand Up @@ -1871,7 +1885,7 @@ static int snd_seq_ioctl_get_client_pool(struct snd_seq_client *client,
struct snd_seq_client_pool *info = arg;
struct snd_seq_client *cptr;

cptr = snd_seq_client_use_ptr(info->client);
cptr = client_load_and_use_ptr(info->client);
if (cptr == NULL)
return -ENOENT;
memset(info, 0, sizeof(*info));
Expand Down Expand Up @@ -1975,7 +1989,7 @@ static int snd_seq_ioctl_get_subscription(struct snd_seq_client *client,
struct snd_seq_client_port *sport = NULL;

result = -EINVAL;
sender = snd_seq_client_use_ptr(subs->sender.client);
sender = client_load_and_use_ptr(subs->sender.client);
if (!sender)
goto __end;
sport = snd_seq_port_use_ptr(sender, subs->sender.port);
Expand Down Expand Up @@ -2006,7 +2020,7 @@ static int snd_seq_ioctl_query_subs(struct snd_seq_client *client, void *arg)
struct list_head *p;
int i;

cptr = snd_seq_client_use_ptr(subs->root.client);
cptr = client_load_and_use_ptr(subs->root.client);
if (!cptr)
goto __end;
port = snd_seq_port_use_ptr(cptr, subs->root.port);
Expand Down Expand Up @@ -2073,7 +2087,7 @@ static int snd_seq_ioctl_query_next_client(struct snd_seq_client *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);
cptr = client_load_and_use_ptr(info->client);
if (cptr)
break; /* found */
}
Expand All @@ -2096,7 +2110,7 @@ static int snd_seq_ioctl_query_next_port(struct snd_seq_client *client,
struct snd_seq_client *cptr;
struct snd_seq_client_port *port = NULL;

cptr = snd_seq_client_use_ptr(info->addr.client);
cptr = client_load_and_use_ptr(info->addr.client);
if (cptr == NULL)
return -ENXIO;

Expand Down Expand Up @@ -2193,7 +2207,7 @@ static int snd_seq_ioctl_client_ump_info(struct snd_seq_client *caller,
size = sizeof(struct snd_ump_endpoint_info);
else
size = sizeof(struct snd_ump_block_info);
cptr = snd_seq_client_use_ptr(client);
cptr = client_load_and_use_ptr(client);
if (!cptr)
return -ENOENT;

Expand Down Expand Up @@ -2475,7 +2489,7 @@ int snd_seq_kernel_client_enqueue(int client, struct snd_seq_event *ev,
if (check_event_type_and_length(ev))
return -EINVAL;

cptr = snd_seq_client_use_ptr(client);
cptr = client_load_and_use_ptr(client);
if (cptr == NULL)
return -EINVAL;

Expand Down Expand Up @@ -2707,7 +2721,7 @@ void snd_seq_info_clients_read(struct snd_info_entry *entry,

/* list the client table */
for (c = 0; c < SNDRV_SEQ_MAX_CLIENTS; c++) {
client = snd_seq_client_use_ptr(c);
client = client_load_and_use_ptr(c);
if (client == NULL)
continue;
if (client->type == NO_CLIENT) {
Expand Down

0 comments on commit c9ce148

Please sign in to comment.