From d7bf73809849463f76de42aad62c850305dd6c5d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 13 Feb 2024 14:53:43 +0100 Subject: ALSA: seq: fix function cast warnings clang-16 points out a control flow integrity (kcfi) issue when event callbacks get converted to incompatible types: sound/core/seq/seq_midi.c:135:30: error: cast from 'int (*)(struct snd_rawmidi_substream *, const char *, int)' to 'snd_seq_dump_func_t' (aka 'int (*)(void *, void *, int)') converts to incompatible function type [-Werror,-Wcast-function-type-strict] 135 | snd_seq_dump_var_event(ev, (snd_seq_dump_func_t)dump_midi, substream); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ sound/core/seq/seq_virmidi.c:83:31: error: cast from 'int (*)(struct snd_rawmidi_substream *, const unsigned char *, int)' to 'snd_seq_dump_func_t' (aka 'int (*)(void *, void *, int)') converts to incompatible function type [-Werror,-Wcast-function-type-strict] 83 | snd_seq_dump_var_event(ev, (snd_seq_dump_func_t)snd_rawmidi_receive, vmidi->substream); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ For addressing those errors, introduce wrapper functions that are used for callbacks and bridge to the actual function call with pointer cast. The code was originally added with the initial ALSA merge in linux-2.5.4. [ the patch description shamelessly copied from Arnd's original patch -- tiwai ] Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Reported-by: Arnd Bergmann Link: https://lore.kernel.org/r/20240213101020.459183-1-arnd@kernel.org Link: https://lore.kernel.org/r/20240213135343.16411-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- sound/core/seq/seq_midi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'sound/core/seq/seq_midi.c') diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c index 18320a248aa7..78dcb0ea1558 100644 --- a/sound/core/seq/seq_midi.c +++ b/sound/core/seq/seq_midi.c @@ -113,6 +113,12 @@ static int dump_midi(struct snd_rawmidi_substream *substream, const char *buf, i return 0; } +/* callback for snd_seq_dump_var_event(), bridging to dump_midi() */ +static int __dump_midi(void *ptr, void *buf, int count) +{ + return dump_midi(ptr, buf, count); +} + static int event_process_midi(struct snd_seq_event *ev, int direct, void *private_data, int atomic, int hop) { @@ -132,7 +138,7 @@ static int event_process_midi(struct snd_seq_event *ev, int direct, pr_debug("ALSA: seq_midi: invalid sysex event flags = 0x%x\n", ev->flags); return 0; } - snd_seq_dump_var_event(ev, (snd_seq_dump_func_t)dump_midi, substream); + snd_seq_dump_var_event(ev, __dump_midi, substream); snd_midi_event_reset_decode(msynth->parser); } else { if (msynth->parser == NULL) -- cgit v1.2.3 From edbcf872c14690c2894b726b67a871b4d238eea8 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 22 Feb 2024 12:15:09 +0100 Subject: ALSA: seq: core: Use automatic cleanup of kfree() There are common patterns where a temporary buffer is allocated and freed at the exit, and those can be simplified with the recent cleanup mechanism via __free(kfree). No functional changes, only code refactoring. Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20240222111509.28390-10-tiwai@suse.de --- sound/core/seq/seq_midi.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) (limited to 'sound/core/seq/seq_midi.c') diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c index 78dcb0ea1558..0594269d92ab 100644 --- a/sound/core/seq/seq_midi.c +++ b/sound/core/seq/seq_midi.c @@ -270,8 +270,8 @@ snd_seq_midisynth_probe(struct device *_dev) struct snd_seq_device *dev = to_seq_dev(_dev); struct seq_midisynth_client *client; struct seq_midisynth *msynth, *ms; - struct snd_seq_port_info *port; - struct snd_rawmidi_info *info; + struct snd_seq_port_info *port __free(kfree) = NULL; + struct snd_rawmidi_info *info __free(kfree) = NULL; struct snd_rawmidi *rmidi = dev->private_data; int newclient = 0; unsigned int p, ports; @@ -297,10 +297,8 @@ snd_seq_midisynth_probe(struct device *_dev) ports = output_count; if (ports < input_count) ports = input_count; - if (ports == 0) { - kfree(info); + if (ports == 0) return -ENODEV; - } if (ports > (256 / SNDRV_RAWMIDI_DEVICES)) ports = 256 / SNDRV_RAWMIDI_DEVICES; @@ -311,7 +309,6 @@ snd_seq_midisynth_probe(struct device *_dev) client = kzalloc(sizeof(*client), GFP_KERNEL); if (client == NULL) { mutex_unlock(®ister_mutex); - kfree(info); return -ENOMEM; } client->seq_client = @@ -321,7 +318,6 @@ snd_seq_midisynth_probe(struct device *_dev) if (client->seq_client < 0) { kfree(client); mutex_unlock(®ister_mutex); - kfree(info); return -ENOMEM; } } @@ -403,8 +399,6 @@ snd_seq_midisynth_probe(struct device *_dev) if (newclient) synths[card->number] = client; mutex_unlock(®ister_mutex); - kfree(info); - kfree(port); return 0; /* success */ __nomem: @@ -417,8 +411,6 @@ snd_seq_midisynth_probe(struct device *_dev) snd_seq_delete_kernel_client(client->seq_client); kfree(client); } - kfree(info); - kfree(port); mutex_unlock(®ister_mutex); return -ENOMEM; } -- cgit v1.2.3 From 45bab301d80b94790c1d9e98ac903aeb373d5f36 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 27 Feb 2024 09:52:59 +0100 Subject: ALSA: seq: midi: Use guard() for locking We can simplify the code gracefully with new guard() macro and co for automatic cleanup of locks. Only the code refactoring, and no functional changes. Signed-off-by: Takashi Iwai Link: https://lore.kernel.org/r/20240227085306.9764-18-tiwai@suse.de --- sound/core/seq/seq_midi.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) (limited to 'sound/core/seq/seq_midi.c') diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c index 0594269d92ab..ba52a77eda38 100644 --- a/sound/core/seq/seq_midi.c +++ b/sound/core/seq/seq_midi.c @@ -302,22 +302,19 @@ snd_seq_midisynth_probe(struct device *_dev) if (ports > (256 / SNDRV_RAWMIDI_DEVICES)) ports = 256 / SNDRV_RAWMIDI_DEVICES; - mutex_lock(®ister_mutex); + guard(mutex)(®ister_mutex); client = synths[card->number]; if (client == NULL) { newclient = 1; client = kzalloc(sizeof(*client), GFP_KERNEL); - if (client == NULL) { - mutex_unlock(®ister_mutex); + if (client == NULL) return -ENOMEM; - } client->seq_client = snd_seq_create_kernel_client( card, 0, "%s", card->shortname[0] ? (const char *)card->shortname : "External MIDI"); if (client->seq_client < 0) { kfree(client); - mutex_unlock(®ister_mutex); return -ENOMEM; } } @@ -398,7 +395,6 @@ snd_seq_midisynth_probe(struct device *_dev) client->num_ports++; if (newclient) synths[card->number] = client; - mutex_unlock(®ister_mutex); return 0; /* success */ __nomem: @@ -411,7 +407,6 @@ snd_seq_midisynth_probe(struct device *_dev) snd_seq_delete_kernel_client(client->seq_client); kfree(client); } - mutex_unlock(®ister_mutex); return -ENOMEM; } @@ -425,12 +420,10 @@ snd_seq_midisynth_remove(struct device *_dev) struct snd_card *card = dev->card; int device = dev->device, p, ports; - mutex_lock(®ister_mutex); + guard(mutex)(®ister_mutex); client = synths[card->number]; - if (client == NULL || client->ports[device] == NULL) { - mutex_unlock(®ister_mutex); + if (client == NULL || client->ports[device] == NULL) return -ENODEV; - } ports = client->ports_per_device[device]; client->ports_per_device[device] = 0; msynth = client->ports[device]; @@ -444,7 +437,6 @@ snd_seq_midisynth_remove(struct device *_dev) synths[card->number] = NULL; kfree(client); } - mutex_unlock(®ister_mutex); return 0; } -- cgit v1.2.3