Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()

[Alexandra Winter]

On 02.11.22 20:09, Kees Cook wrote:
> On Wed, Nov 02, 2022 at 09:32:50AM -0700, Nathan Chancellor wrote:
>> With clang's kernel control flow integrity (kCFI, CONFIG_CFI_CLANG),
>> indirect call targets are validated against the expected function
>> pointer prototype to make sure the call target is valid to help mitigate
>> ROP attacks. If they are not identical, there is a failure at run time,
>> which manifests as either a kernel panic or thread getting killed. A
>> proposed warning in clang aims to catch these at compile time, which
>> reveals:
>>
>>   drivers/s390/net/ctcm_main.c:1064:21: error: incompatible function pointer types initializing 'netdev_tx_t (*)(struct sk_buff *, struct net_device *)' (aka 'enum netdev_tx (*)(struct sk_buff *, struct net_device *)') with an expression of type 'int (struct sk_buff *, struct net_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
>>           .ndo_start_xmit         = ctcm_tx,
>>                                     ^~~~~~~
>>   drivers/s390/net/ctcm_main.c:1072:21: error: incompatible function pointer types initializing 'netdev_tx_t (*)(struct sk_buff *, struct net_device *)' (aka 'enum netdev_tx (*)(struct sk_buff *, struct net_device *)') with an expression of type 'int (struct sk_buff *, struct net_device *)' [-Werror,-Wincompatible-function-pointer-types-strict]
>>           .ndo_start_xmit         = ctcmpc_tx,
>>                                     ^~~~~~~~~
>>
>> ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
>> 'netdev_tx_t', not 'int'. Adjust the return type of ctc{mp,}m_tx() to
>> match the prototype's to resolve the warning and potential CFI failure,
>> should s390 select ARCH_SUPPORTS_CFI_CLANG in the future.
>>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/1750
>> Signed-off-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> 
> Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>
> 

Could you please also remove the corresponding comments:
diff --git a/drivers/s390/net/ctcm_main.c b/drivers/s390/net/ctcm_main.c
index 37b551bd43bf..14200548704a 100644
--- a/drivers/s390/net/ctcm_main.c
+++ b/drivers/s390/net/ctcm_main.c
@@ -825,13 +825,6 @@ static int ctcmpc_transmit_skb(struct channel *ch, struct sk_buff *skb)
 /*
  * Start transmission of a packet.
  * Called from generic network device layer.
- *
- *  skb                Pointer to buffer containing the packet.
- *  dev                Pointer to interface struct.
- *
- * returns 0 if packet consumed, !0 if packet rejected.
- *         Note: If we return !0, then the packet is free'd by
- *               the generic network layer.
  */
 /* first merge version - leaving both functions separated */
 static int ctcm_tx(struct sk_buff *skb, struct net_device *dev)

Reviewed-by: Alexandra Winter <wintera@xxxxxxxxxxxxx>