2020-05-16 17:45:30

by Oscar Carter

[permalink] [raw]
Subject: [PATCH] firewire: Remove function callback casts

In an effort to enable -Wcast-function-type in the top-level Makefile to
support Control Flow Integrity builds, remove all the function callback
casts.

To do this, modify the "fw_iso_context_create" function prototype adding
a new parameter for the multichannel callback. Also, fix all the
function calls accordingly.

Signed-off-by: Oscar Carter <[email protected]>
---
drivers/firewire/core-cdev.c | 12 +++++++-----
drivers/firewire/core-iso.c | 10 ++++++++--
drivers/firewire/net.c | 2 +-
drivers/media/firewire/firedtv-fw.c | 3 ++-
include/linux/firewire.h | 3 ++-
sound/firewire/amdtp-stream.c | 2 +-
sound/firewire/isight.c | 4 ++--
7 files changed, 23 insertions(+), 13 deletions(-)

diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 6e291d8f3a27..cc368b35be2e 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -957,7 +957,8 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
{
struct fw_cdev_create_iso_context *a = &arg->create_iso_context;
struct fw_iso_context *context;
- fw_iso_callback_t cb;
+ fw_iso_callback_t cb_sc = NULL;
+ fw_iso_mc_callback_t cb_mc = NULL;
int ret;

BUILD_BUG_ON(FW_CDEV_ISO_CONTEXT_TRANSMIT != FW_ISO_CONTEXT_TRANSMIT ||
@@ -970,7 +971,7 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
if (a->speed > SCODE_3200 || a->channel > 63)
return -EINVAL;

- cb = iso_callback;
+ cb_sc = iso_callback;
break;

case FW_ISO_CONTEXT_RECEIVE:
@@ -978,11 +979,11 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
a->channel > 63)
return -EINVAL;

- cb = iso_callback;
+ cb_sc = iso_callback;
break;

case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
- cb = (fw_iso_callback_t)iso_mc_callback;
+ cb_mc = iso_mc_callback;
break;

default:
@@ -990,7 +991,8 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
}

context = fw_iso_context_create(client->device->card, a->type,
- a->channel, a->speed, a->header_size, cb, client);
+ a->channel, a->speed, a->header_size, cb_sc, cb_mc,
+ client);
if (IS_ERR(context))
return PTR_ERR(context);
if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
index 185b0b78b3d6..3b8e349704f8 100644
--- a/drivers/firewire/core-iso.c
+++ b/drivers/firewire/core-iso.c
@@ -131,7 +131,8 @@ size_t fw_iso_buffer_lookup(struct fw_iso_buffer *buffer, dma_addr_t completed)

struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
int type, int channel, int speed, size_t header_size,
- fw_iso_callback_t callback, void *callback_data)
+ fw_iso_callback_t cb_sc, fw_iso_mc_callback_t cb_mc,
+ void *callback_data)
{
struct fw_iso_context *ctx;

@@ -145,7 +146,12 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
ctx->channel = channel;
ctx->speed = speed;
ctx->header_size = header_size;
- ctx->callback.sc = callback;
+
+ if (cb_sc)
+ ctx->callback.sc = cb_sc;
+ else
+ ctx->callback.mc = cb_mc;
+
ctx->callback_data = callback_data;

return ctx;
diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
index 715e491dfbc3..c5cc0a311aa0 100644
--- a/drivers/firewire/net.c
+++ b/drivers/firewire/net.c
@@ -1136,7 +1136,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
context = fw_iso_context_create(dev->card, FW_ISO_CONTEXT_RECEIVE,
IEEE1394_BROADCAST_CHANNEL,
dev->card->link_speed, 8,
- fwnet_receive_broadcast, dev);
+ fwnet_receive_broadcast, NULL, dev);
if (IS_ERR(context)) {
retval = PTR_ERR(context);
goto failed;
diff --git a/drivers/media/firewire/firedtv-fw.c b/drivers/media/firewire/firedtv-fw.c
index 97144734eb05..d2940adefd8c 100644
--- a/drivers/media/firewire/firedtv-fw.c
+++ b/drivers/media/firewire/firedtv-fw.c
@@ -141,7 +141,8 @@ int fdtv_start_iso(struct firedtv *fdtv)

ctx->context = fw_iso_context_create(device->card,
FW_ISO_CONTEXT_RECEIVE, fdtv->isochannel,
- device->max_speed, ISO_HEADER_SIZE, handle_iso, fdtv);
+ device->max_speed, ISO_HEADER_SIZE,
+ handle_iso, NULL, fdtv);
if (IS_ERR(ctx->context)) {
err = PTR_ERR(ctx->context);
goto fail_free;
diff --git a/include/linux/firewire.h b/include/linux/firewire.h
index aec8f30ab200..3a0b5e18e140 100644
--- a/include/linux/firewire.h
+++ b/include/linux/firewire.h
@@ -452,7 +452,8 @@ struct fw_iso_context {

struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
int type, int channel, int speed, size_t header_size,
- fw_iso_callback_t callback, void *callback_data);
+ fw_iso_callback_t cb_sc, fw_iso_mc_callback_t cb_mc,
+ void *callback_data);
int fw_iso_context_set_channels(struct fw_iso_context *ctx, u64 *channels);
int fw_iso_context_queue(struct fw_iso_context *ctx,
struct fw_iso_packet *packet,
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 37d38efb4c87..8629ab3e2c64 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -1093,7 +1093,7 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed,

s->context = fw_iso_context_create(fw_parent_device(s->unit)->card,
type, channel, speed, ctx_header_size,
- ctx_cb, ctx_data);
+ ctx_cb, NULL, ctx_data);
if (IS_ERR(s->context)) {
err = PTR_ERR(s->context);
if (err == -EBUSY)
diff --git a/sound/firewire/isight.c b/sound/firewire/isight.c
index 6655af53b367..51cc37fca736 100644
--- a/sound/firewire/isight.c
+++ b/sound/firewire/isight.c
@@ -361,8 +361,8 @@ static int isight_start_streaming(struct isight *isight)
isight->context = fw_iso_context_create(isight->device->card,
FW_ISO_CONTEXT_RECEIVE,
isight->resources.channel,
- isight->device->max_speed,
- 4, isight_packet, isight);
+ isight->device->max_speed, 4,
+ isight_packet, NULL, isight);
if (IS_ERR(isight->context)) {
err = PTR_ERR(isight->context);
isight->context = NULL;
--
2.20.1


2020-05-17 16:49:24

by Lev R. Oshvang .

[permalink] [raw]
Subject: Re: [PATCH] firewire: Remove function callback casts

On Sat, May 16, 2020 at 8:40 PM Oscar Carter <[email protected]> wrote:
>
> In an effort to enable -Wcast-function-type in the top-level Makefile to
> support Control Flow Integrity builds, remove all the function callback
> casts.
>
> To do this, modify the "fw_iso_context_create" function prototype adding
> a new parameter for the multichannel callback. Also, fix all the
> function calls accordingly.
>
> Signed-off-by: Oscar Carter <[email protected]>
> ---
> drivers/firewire/core-cdev.c | 12 +++++++-----
> drivers/firewire/core-iso.c | 10 ++++++++--
> drivers/firewire/net.c | 2 +-
> drivers/media/firewire/firedtv-fw.c | 3 ++-
> include/linux/firewire.h | 3 ++-
> sound/firewire/amdtp-stream.c | 2 +-
> sound/firewire/isight.c | 4 ++--
> 7 files changed, 23 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
> index 6e291d8f3a27..cc368b35be2e 100644
> --- a/drivers/firewire/core-cdev.c
> +++ b/drivers/firewire/core-cdev.c
> @@ -957,7 +957,8 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
> {
> struct fw_cdev_create_iso_context *a = &arg->create_iso_context;
> struct fw_iso_context *context;
> - fw_iso_callback_t cb;
> + fw_iso_callback_t cb_sc = NULL;
> + fw_iso_mc_callback_t cb_mc = NULL;
> int ret;
>
> BUILD_BUG_ON(FW_CDEV_ISO_CONTEXT_TRANSMIT != FW_ISO_CONTEXT_TRANSMIT ||
> @@ -970,7 +971,7 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
> if (a->speed > SCODE_3200 || a->channel > 63)
> return -EINVAL;
>
> - cb = iso_callback;
> + cb_sc = iso_callback;
> break;
>
> case FW_ISO_CONTEXT_RECEIVE:
> @@ -978,11 +979,11 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
> a->channel > 63)
> return -EINVAL;
>
> - cb = iso_callback;
> + cb_sc = iso_callback;
> break;
>
> case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL:
> - cb = (fw_iso_callback_t)iso_mc_callback;
> + cb_mc = iso_mc_callback;
> break;
>
> default:
> @@ -990,7 +991,8 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg)
> }
>
> context = fw_iso_context_create(client->device->card, a->type,
> - a->channel, a->speed, a->header_size, cb, client);
> + a->channel, a->speed, a->header_size, cb_sc, cb_mc,
> + client);
> if (IS_ERR(context))
> return PTR_ERR(context);
> if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
> diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c
> index 185b0b78b3d6..3b8e349704f8 100644
> --- a/drivers/firewire/core-iso.c
> +++ b/drivers/firewire/core-iso.c
> @@ -131,7 +131,8 @@ size_t fw_iso_buffer_lookup(struct fw_iso_buffer *buffer, dma_addr_t completed)
>
> struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
> int type, int channel, int speed, size_t header_size,
> - fw_iso_callback_t callback, void *callback_data)
> + fw_iso_callback_t cb_sc, fw_iso_mc_callback_t cb_mc,
> + void *callback_data)
> {
> struct fw_iso_context *ctx;
>
> @@ -145,7 +146,12 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
> ctx->channel = channel;
> ctx->speed = speed;
> ctx->header_size = header_size;
> - ctx->callback.sc = callback;
> +
> + if (cb_sc)
> + ctx->callback.sc = cb_sc;
> + else
> + ctx->callback.mc = cb_mc;
> +
> ctx->callback_data = callback_data;
>
> return ctx;
> diff --git a/drivers/firewire/net.c b/drivers/firewire/net.c
> index 715e491dfbc3..c5cc0a311aa0 100644
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
> @@ -1136,7 +1136,7 @@ static int fwnet_broadcast_start(struct fwnet_device *dev)
> context = fw_iso_context_create(dev->card, FW_ISO_CONTEXT_RECEIVE,
> IEEE1394_BROADCAST_CHANNEL,
> dev->card->link_speed, 8,
> - fwnet_receive_broadcast, dev);
> + fwnet_receive_broadcast, NULL, dev);
> if (IS_ERR(context)) {
> retval = PTR_ERR(context);
> goto failed;
> diff --git a/drivers/media/firewire/firedtv-fw.c b/drivers/media/firewire/firedtv-fw.c
> index 97144734eb05..d2940adefd8c 100644
> --- a/drivers/media/firewire/firedtv-fw.c
> +++ b/drivers/media/firewire/firedtv-fw.c
> @@ -141,7 +141,8 @@ int fdtv_start_iso(struct firedtv *fdtv)
>
> ctx->context = fw_iso_context_create(device->card,
> FW_ISO_CONTEXT_RECEIVE, fdtv->isochannel,
> - device->max_speed, ISO_HEADER_SIZE, handle_iso, fdtv);
> + device->max_speed, ISO_HEADER_SIZE,
> + handle_iso, NULL, fdtv);
> if (IS_ERR(ctx->context)) {
> err = PTR_ERR(ctx->context);
> goto fail_free;
> diff --git a/include/linux/firewire.h b/include/linux/firewire.h
> index aec8f30ab200..3a0b5e18e140 100644
> --- a/include/linux/firewire.h
> +++ b/include/linux/firewire.h
> @@ -452,7 +452,8 @@ struct fw_iso_context {
>
> struct fw_iso_context *fw_iso_context_create(struct fw_card *card,
> int type, int channel, int speed, size_t header_size,
> - fw_iso_callback_t callback, void *callback_data);
> + fw_iso_callback_t cb_sc, fw_iso_mc_callback_t cb_mc,
> + void *callback_data);
> int fw_iso_context_set_channels(struct fw_iso_context *ctx, u64 *channels);
> int fw_iso_context_queue(struct fw_iso_context *ctx,
> struct fw_iso_packet *packet,
> diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
> index 37d38efb4c87..8629ab3e2c64 100644
> --- a/sound/firewire/amdtp-stream.c
> +++ b/sound/firewire/amdtp-stream.c
> @@ -1093,7 +1093,7 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed,
>
> s->context = fw_iso_context_create(fw_parent_device(s->unit)->card,
> type, channel, speed, ctx_header_size,
> - ctx_cb, ctx_data);
> + ctx_cb, NULL, ctx_data);
> if (IS_ERR(s->context)) {
> err = PTR_ERR(s->context);
> if (err == -EBUSY)
> diff --git a/sound/firewire/isight.c b/sound/firewire/isight.c
> index 6655af53b367..51cc37fca736 100644
> --- a/sound/firewire/isight.c
> +++ b/sound/firewire/isight.c
> @@ -361,8 +361,8 @@ static int isight_start_streaming(struct isight *isight)
> isight->context = fw_iso_context_create(isight->device->card,
> FW_ISO_CONTEXT_RECEIVE,
> isight->resources.channel,
> - isight->device->max_speed,
> - 4, isight_packet, isight);
> + isight->device->max_speed, 4,
> + isight_packet, NULL, isight);
> if (IS_ERR(isight->context)) {
> err = PTR_ERR(isight->context);
> isight->context = NULL;
> --
> 2.20.1
>

Hi Oscar,
I would recommend to explicitly set NULL to _cb and use if elif to
exclude the case where botm _cb paraneters are passed as NULLs :
> +
ctx->callback.mc =NULL:
ctx->callback.sc = NULL;
> + if (cb_sc)
> + ctx->callback.sc = cb_sc;

> + elif (cb_ms)
> + ctx->callback.mc = cb_mc;
> + else
return -EINVAL;

Hope you get the point.
Lev

2020-05-19 17:04:54

by Oscar Carter

[permalink] [raw]
Subject: Re: [PATCH] firewire: Remove function callback casts

On Sun, May 17, 2020 at 07:43:21PM +0300, Lev R. Oshvang . wrote:
>
> Hi Oscar,
> I would recommend to explicitly set NULL to _cb and use if elif to
> exclude the case where botm _cb paraneters are passed as NULLs :
> > +
> ctx->callback.mc =NULL:
> ctx->callback.sc = NULL;
> > + if (cb_sc)
> > + ctx->callback.sc = cb_sc;
>
> > + elif (cb_ms)
> > + ctx->callback.mc = cb_mc;
> > + else
> return -EINVAL;

Ok, I will do the changes you suggested and I will resend a new version.

> Hope you get the point.
> Lev

Thanks,
Oscar Carter