| Message ID | 20240213101020.459183-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: fix function cast warnings | expand |
On Tue, 13 Feb 2024 11:09:56 +0100, Arnd Bergmann wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > 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); > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > Change these both to take a 'const void *' buffer and a 'void *' context, > converting to the respective types in the callee. The change to 'const' > buffers propagates to a couple of other functions. > > The code was originally added with the initial ALSA merge in linux-2.5.4. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > include/sound/rawmidi.h | 3 +-- > include/sound/seq_kernel.h | 2 +- > sound/core/rawmidi.c | 6 +++--- > sound/core/seq/oss/seq_oss_readq.c | 4 ++-- > sound/core/seq/oss/seq_oss_readq.h | 2 +- > sound/core/seq/seq_memory.c | 4 ++-- > sound/core/seq/seq_midi.c | 5 +++-- > sound/core/seq/seq_virmidi.c | 2 +- > 8 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h > index f31cabf0158c..91947fb16e07 100644 > --- a/include/sound/rawmidi.h > +++ b/include/sound/rawmidi.h > @@ -161,8 +161,7 @@ int snd_rawmidi_free(struct snd_rawmidi *rmidi); > > /* callbacks */ > > -int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, > - const unsigned char *buffer, int count); > +int snd_rawmidi_receive(void *ptr, const void *buffer, int count); If it were only about the type of the buffer argument being a void pointer, it's fine. But the substream argument should be explicitly typed, otherwise it's confusing for other normal call patterns. I guess the suitable fix for now would be to provide wrapper functions that are used for callbacks and bridge to the actual function with pointer cast, something like below. Eventually we can put more const, but it's basically irrelevant with the warning itself. thanks, Takashi -- 8< -- --- 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) --- a/sound/core/seq/seq_virmidi.c +++ b/sound/core/seq/seq_virmidi.c @@ -62,6 +62,13 @@ static void snd_virmidi_init_event(struct snd_virmidi *vmidi, /* * decode input event and put to read buffer of each opened file */ + +/* callback for snd_seq_dump_var_event(), bridging to snd_rawmidi_receive() */ +static int dump_to_rawmidi(void *ptr, void *buf, int count) +{ + return snd_rawmidi_receive(ptr, buf, count); +} + static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev, struct snd_seq_event *ev, bool atomic) @@ -80,7 +87,7 @@ static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev, if (ev->type == SNDRV_SEQ_EVENT_SYSEX) { if ((ev->flags & SNDRV_SEQ_EVENT_LENGTH_MASK) != SNDRV_SEQ_EVENT_LENGTH_VARIABLE) continue; - snd_seq_dump_var_event(ev, (snd_seq_dump_func_t)snd_rawmidi_receive, vmidi->substream); + snd_seq_dump_var_event(ev, dump_to_rawmidi, vmidi->substream); snd_midi_event_reset_decode(vmidi->parser); } else { len = snd_midi_event_decode(vmidi->parser, msg, sizeof(msg), ev);
On Tue, Feb 13, 2024, at 13:56, Takashi Iwai wrote: > On Tue, 13 Feb 2024 11:09:56 +0100, >> >> -int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, >> - const unsigned char *buffer, int count); >> +int snd_rawmidi_receive(void *ptr, const void *buffer, int count); > > If it were only about the type of the buffer argument being a void > pointer, it's fine. But the substream argument should be explicitly > typed, otherwise it's confusing for other normal call patterns. > > I guess the suitable fix for now would be to provide wrapper functions > that are used for callbacks and bridge to the actual function with > pointer cast, something like below. Eventually we can put more const, > but it's basically irrelevant with the warning itself. Right, makes sense. I gave your patch a spin and it addresses the warnings I saw in randconfig builds, so please apply that with "Reported-by: Arnd Bergmann <arnd@arndb.de>". Arnd
On Tue, 13 Feb 2024 14:30:56 +0100, Arnd Bergmann wrote: > > On Tue, Feb 13, 2024, at 13:56, Takashi Iwai wrote: > > On Tue, 13 Feb 2024 11:09:56 +0100, > > >> > >> -int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, > >> - const unsigned char *buffer, int count); > >> +int snd_rawmidi_receive(void *ptr, const void *buffer, int count); > > > > If it were only about the type of the buffer argument being a void > > pointer, it's fine. But the substream argument should be explicitly > > typed, otherwise it's confusing for other normal call patterns. > > > > I guess the suitable fix for now would be to provide wrapper functions > > that are used for callbacks and bridge to the actual function with > > pointer cast, something like below. Eventually we can put more const, > > but it's basically irrelevant with the warning itself. > > Right, makes sense. I gave your patch a spin and it addresses the > warnings I saw in randconfig builds, so please apply that with > "Reported-by: Arnd Bergmann <arnd@arndb.de>". OK, will do. thanks, Takashi
diff --git a/include/sound/rawmidi.h b/include/sound/rawmidi.h index f31cabf0158c..91947fb16e07 100644 --- a/include/sound/rawmidi.h +++ b/include/sound/rawmidi.h @@ -161,8 +161,7 @@ int snd_rawmidi_free(struct snd_rawmidi *rmidi); /* callbacks */ -int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, - const unsigned char *buffer, int count); +int snd_rawmidi_receive(void *ptr, const void *buffer, int count); int snd_rawmidi_transmit_empty(struct snd_rawmidi_substream *substream); int snd_rawmidi_transmit_peek(struct snd_rawmidi_substream *substream, unsigned char *buffer, int count); diff --git a/include/sound/seq_kernel.h b/include/sound/seq_kernel.h index c8621671fa70..804194d7c606 100644 --- a/include/sound/seq_kernel.h +++ b/include/sound/seq_kernel.h @@ -67,7 +67,7 @@ int snd_seq_kernel_client_ctl(int client, unsigned int cmd, void *arg); #define SNDRV_SEQ_EXT_USRPTR 0x80000000 #define SNDRV_SEQ_EXT_CHAINED 0x40000000 -typedef int (*snd_seq_dump_func_t)(void *ptr, void *buf, int count); +typedef int (*snd_seq_dump_func_t)(void *ptr, const void *buf, int count); int snd_seq_expand_var_event(const struct snd_seq_event *event, int count, char *buf, int in_kernel, int size_aligned); int snd_seq_expand_var_event_at(const struct snd_seq_event *event, int count, diff --git a/sound/core/rawmidi.c b/sound/core/rawmidi.c index 1431cb997808..ad588dcf857f 100644 --- a/sound/core/rawmidi.c +++ b/sound/core/rawmidi.c @@ -1165,9 +1165,9 @@ static struct timespec64 get_framing_tstamp(struct snd_rawmidi_substream *substr * * Return: The size of read data, or a negative error code on failure. */ -int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, - const unsigned char *buffer, int count) +int snd_rawmidi_receive(void *ptr, const void *buffer, int count) { + struct snd_rawmidi_substream *substream = ptr; unsigned long flags; struct timespec64 ts64 = get_framing_tstamp(substream); int result = 0, count1; @@ -1195,7 +1195,7 @@ int snd_rawmidi_receive(struct snd_rawmidi_substream *substream, } else if (count == 1) { /* special case, faster code */ substream->bytes++; if (runtime->avail < runtime->buffer_size) { - runtime->buffer[runtime->hw_ptr++] = buffer[0]; + runtime->buffer[runtime->hw_ptr++] = ((u8 *)buffer)[0]; runtime->hw_ptr %= runtime->buffer_size; runtime->avail++; result++; diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c index f0db5d3dcba4..3679938c0ad9 100644 --- a/sound/core/seq/oss/seq_oss_readq.c +++ b/sound/core/seq/oss/seq_oss_readq.c @@ -86,7 +86,7 @@ snd_seq_oss_readq_clear(struct seq_oss_readq *q) * put a midi byte */ int -snd_seq_oss_readq_puts(struct seq_oss_readq *q, int dev, unsigned char *data, int len) +snd_seq_oss_readq_puts(struct seq_oss_readq *q, int dev, const unsigned char *data, int len) { union evrec rec; int result; @@ -113,7 +113,7 @@ struct readq_sysex_ctx { int dev; }; -static int readq_dump_sysex(void *ptr, void *buf, int count) +static int readq_dump_sysex(void *ptr, const void *buf, int count) { struct readq_sysex_ctx *ctx = ptr; diff --git a/sound/core/seq/oss/seq_oss_readq.h b/sound/core/seq/oss/seq_oss_readq.h index 38d0c4682b29..bb99743fec7d 100644 --- a/sound/core/seq/oss/seq_oss_readq.h +++ b/sound/core/seq/oss/seq_oss_readq.h @@ -30,7 +30,7 @@ struct seq_oss_readq *snd_seq_oss_readq_new(struct seq_oss_devinfo *dp, int maxl void snd_seq_oss_readq_delete(struct seq_oss_readq *q); void snd_seq_oss_readq_clear(struct seq_oss_readq *readq); unsigned int snd_seq_oss_readq_poll(struct seq_oss_readq *readq, struct file *file, poll_table *wait); -int snd_seq_oss_readq_puts(struct seq_oss_readq *readq, int dev, unsigned char *data, int len); +int snd_seq_oss_readq_puts(struct seq_oss_readq *readq, int dev, const unsigned char *data, int len); int snd_seq_oss_readq_sysex(struct seq_oss_readq *q, int dev, struct snd_seq_event *ev); int snd_seq_oss_readq_put_event(struct seq_oss_readq *readq, union evrec *ev); diff --git a/sound/core/seq/seq_memory.c b/sound/core/seq/seq_memory.c index e705e7538118..3be3ee178e20 100644 --- a/sound/core/seq/seq_memory.c +++ b/sound/core/seq/seq_memory.c @@ -135,7 +135,7 @@ EXPORT_SYMBOL(snd_seq_dump_var_event); * expand the variable length event to linear buffer space. */ -static int seq_copy_in_kernel(void *ptr, void *src, int size) +static int seq_copy_in_kernel(void *ptr, const void *src, int size) { char **bufptr = ptr; @@ -144,7 +144,7 @@ static int seq_copy_in_kernel(void *ptr, void *src, int size) return 0; } -static int seq_copy_in_user(void *ptr, void *src, int size) +static int seq_copy_in_user(void *ptr, const void *src, int size) { char __user **bufptr = ptr; diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c index 18320a248aa7..e6547da31b09 100644 --- a/sound/core/seq/seq_midi.c +++ b/sound/core/seq/seq_midi.c @@ -94,8 +94,9 @@ static void snd_midi_input_event(struct snd_rawmidi_substream *substream) } } -static int dump_midi(struct snd_rawmidi_substream *substream, const char *buf, int count) +static int dump_midi(void *ptr, const void *buf, int count) { + struct snd_rawmidi_substream *substream = ptr; struct snd_rawmidi_runtime *runtime; int tmp; @@ -132,7 +133,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) diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c index 1b9260108e48..ef8536408af4 100644 --- a/sound/core/seq/seq_virmidi.c +++ b/sound/core/seq/seq_virmidi.c @@ -80,7 +80,7 @@ static int snd_virmidi_dev_receive_event(struct snd_virmidi_dev *rdev, if (ev->type == SNDRV_SEQ_EVENT_SYSEX) { if ((ev->flags & SNDRV_SEQ_EVENT_LENGTH_MASK) != SNDRV_SEQ_EVENT_LENGTH_VARIABLE) continue; - snd_seq_dump_var_event(ev, (snd_seq_dump_func_t)snd_rawmidi_receive, vmidi->substream); + snd_seq_dump_var_event(ev, snd_rawmidi_receive, vmidi->substream); snd_midi_event_reset_decode(vmidi->parser); } else { len = snd_midi_event_decode(vmidi->parser, msg, sizeof(msg), ev);