[PATCH 0/2] firewire: obsolete cast of function callback toward CFI
Hi,
Oscar Carter works for Control Flow Integrity build. Any cast of function callback is inconvenient for the work. Unfortunately, current code of firewire-core driver includes the cast[1] and Oscar posted some patches to remove it[2]. The patch is itself good. However, it includes changes existent kernel API and all of drivers as user of the API get affects from the change.
This patchset is an alternative idea to add a new kernel API specific for multichannel isoc context. The existent kernel API and drivers is left as is.
Practically, no in-kernel drivers use the additional API. Although the API is exported in the patchset, it's better to discuss about unexporting the API.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv... [2] https://lore.kernel.org/lkml/20200519173425.4724-1-oscar.carter@gmx.com/
Regards
Takashi Sakamoto (2): firewire-core: add kernel API to construct multichannel isoc context firewire-core: obsolete cast of function callback
drivers/firewire/core-cdev.c | 44 +++++++++++++++++++----------------- drivers/firewire/core-iso.c | 17 ++++++++++++++ include/linux/firewire.h | 3 +++ 3 files changed, 43 insertions(+), 21 deletions(-)
In 1394 OHCI specification, IR context has several modes. One of mode is 'multiChanMode'. For this mode, Linux FireWire stack has FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL flag apart from FW_ISO_CONTEXT_RECEIVE, and associated internal callback. However, code of firewire-core driver includes cast of function callback for the mode and this brings inconvenient to effort of Control Flow Integrity builds.
This commit is a preparation to remove the cast. A new kernel API for the mode is added and existent API is specific for FW_ISO_CONTEXT_RECEIVE and FW_ISO_CONTEXT_TRANSMIT modes. Actually, no in-kernel driver uses the mode and the additional kernel API is never used at present.
Reported-by: Oscar Carter oscar.carter@gmx.com Reference: https://lore.kernel.org/lkml/20200519173425.4724-1-oscar.carter@gmx.com/ Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- drivers/firewire/core-iso.c | 17 +++++++++++++++++ include/linux/firewire.h | 3 +++ 2 files changed, 20 insertions(+)
diff --git a/drivers/firewire/core-iso.c b/drivers/firewire/core-iso.c index 185b0b78b3d6..07e967594f27 100644 --- a/drivers/firewire/core-iso.c +++ b/drivers/firewire/core-iso.c @@ -152,6 +152,23 @@ struct fw_iso_context *fw_iso_context_create(struct fw_card *card, } EXPORT_SYMBOL(fw_iso_context_create);
+struct fw_iso_context *fw_iso_mc_context_create(struct fw_card *card, + int type, int channel, int speed, size_t header_size, + fw_iso_mc_callback_t callback, void *callback_data) +{ + struct fw_iso_context *ctx; + + ctx = fw_iso_context_create(card, type, channel, speed, header_size, + NULL, callback_data); + if (IS_ERR(ctx)) + return ctx; + + ctx->callback.mc = callback; + + return ctx; +} +EXPORT_SYMBOL(fw_iso_mc_context_create); + void fw_iso_context_destroy(struct fw_iso_context *ctx) { ctx->card->driver->free_iso_context(ctx); diff --git a/include/linux/firewire.h b/include/linux/firewire.h index aec8f30ab200..9477814ab12a 100644 --- a/include/linux/firewire.h +++ b/include/linux/firewire.h @@ -453,6 +453,9 @@ 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); +struct fw_iso_context *fw_iso_mc_context_create(struct fw_card *card, + int type, int channel, int speed, size_t header_size, + fw_iso_mc_callback_t callback, 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,
This commit obsoletes cast of function callback to assist attempt of Control Flow Integrity builds.
Reported-by: Oscar Carter oscar.carter@gmx.com Reference: https://lore.kernel.org/lkml/20200519173425.4724-1-oscar.carter@gmx.com/ Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- drivers/firewire/core-cdev.c | 44 +++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c index 6e291d8f3a27..f1e83396dd22 100644 --- a/drivers/firewire/core-cdev.c +++ b/drivers/firewire/core-cdev.c @@ -957,7 +957,6 @@ 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; int ret;
BUILD_BUG_ON(FW_CDEV_ISO_CONTEXT_TRANSMIT != FW_ISO_CONTEXT_TRANSMIT || @@ -965,32 +964,35 @@ static int ioctl_create_iso_context(struct client *client, union ioctl_arg *arg) FW_CDEV_ISO_CONTEXT_RECEIVE_MULTICHANNEL != FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL);
- switch (a->type) { - case FW_ISO_CONTEXT_TRANSMIT: - if (a->speed > SCODE_3200 || a->channel > 63) - return -EINVAL; - - cb = iso_callback; - break; + if (a->type != FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL) { + fw_iso_callback_t cb;
- case FW_ISO_CONTEXT_RECEIVE: - if (a->header_size < 4 || (a->header_size & 3) || - a->channel > 63) - return -EINVAL; + switch (a->type) { + case FW_ISO_CONTEXT_TRANSMIT: + if (a->speed > SCODE_3200 || a->channel > 63) + return -EINVAL;
- cb = iso_callback; - break; + cb = iso_callback; + break;
- case FW_ISO_CONTEXT_RECEIVE_MULTICHANNEL: - cb = (fw_iso_callback_t)iso_mc_callback; - break; + case FW_ISO_CONTEXT_RECEIVE: + if (a->header_size < 4 || (a->header_size & 3) || + a->channel > 63) + return -EINVAL;
- default: - return -EINVAL; - } + cb = iso_callback; + break; + default: + return -EINVAL; + }
- context = fw_iso_context_create(client->device->card, a->type, + context = fw_iso_context_create(client->device->card, a->type, a->channel, a->speed, a->header_size, cb, client); + } else { + context = fw_iso_mc_context_create(client->device->card, + a->type, a->channel, a->speed, a->header_size, + iso_mc_callback, client); + } if (IS_ERR(context)) return PTR_ERR(context); if (client->version < FW_CDEV_VERSION_AUTO_FLUSH_ISO_OVERFLOW)
Hi,
On Wed, May 20, 2020 at 03:47:24PM +0900, Takashi Sakamoto wrote:
Hi,
Oscar Carter works for Control Flow Integrity build. Any cast of function callback is inconvenient for the work. Unfortunately, current code of firewire-core driver includes the cast[1] and Oscar posted some patches to remove it[2]. The patch is itself good. However, it includes changes existent kernel API and all of drivers as user of the API get affects from the change.
This patchset is an alternative idea to add a new kernel API specific for multichannel isoc context. The existent kernel API and drivers is left as is.
Practically, no in-kernel drivers use the additional API. Although the API is exported in the patchset, it's better to discuss about unexporting the API.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/driv... [2] https://lore.kernel.org/lkml/20200519173425.4724-1-oscar.carter@gmx.com/
Regards
Takashi Sakamoto (2): firewire-core: add kernel API to construct multichannel isoc context firewire-core: obsolete cast of function callback
drivers/firewire/core-cdev.c | 44 +++++++++++++++++++----------------- drivers/firewire/core-iso.c | 17 ++++++++++++++ include/linux/firewire.h | 3 +++ 3 files changed, 43 insertions(+), 21 deletions(-)
-- 2.25.1
Thanks for your work and new proposal. I've done a test build an it cleans the -Wcast-function-type warning without the need to change the current API. So, it looks good to me.
Thanks, Oscar Carter
participants (2)
-
Oscar Carter -
Takashi Sakamoto