* [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx()
2022-11-02 16:32 [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Nathan Chancellor
@ 2022-11-02 16:32 ` Nathan Chancellor
2022-11-02 19:09 ` Kees Cook
2022-11-03 12:09 ` Alexandra Winter
2022-11-02 16:32 ` [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit() Nathan Chancellor
` (2 subsequent siblings)
3 siblings, 2 replies; 13+ messages in thread
From: Nathan Chancellor @ 2022-11-02 16:32 UTC (permalink / raw)
To: Alexandra Winter, Wenjia Zhang
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches, Nathan Chancellor
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/netiucv.c:1854: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 = netiucv_tx,
^~~~~~~~~~
->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
'netdev_tx_t', not 'int'. Adjust the return type of netiucv_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@kernel.org>
---
drivers/s390/net/netiucv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c
index 65aa0a96c21d..1a7f2bc3a87b 100644
--- a/drivers/s390/net/netiucv.c
+++ b/drivers/s390/net/netiucv.c
@@ -1256,7 +1256,7 @@ static int netiucv_close(struct net_device *dev)
* Note: If we return !0, then the packet is free'd by
* the generic network layer.
*/
-static int netiucv_tx(struct sk_buff *skb, struct net_device *dev)
+static netdev_tx_t netiucv_tx(struct sk_buff *skb, struct net_device *dev)
{
struct netiucv_priv *privptr = netdev_priv(dev);
int rc;
--
2.38.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx()
2022-11-02 16:32 ` [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx() Nathan Chancellor
@ 2022-11-02 19:09 ` Kees Cook
2022-11-03 12:09 ` Alexandra Winter
1 sibling, 0 replies; 13+ messages in thread
From: Kees Cook @ 2022-11-02 19:09 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexandra Winter, Wenjia Zhang, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
linux-s390, netdev, Nick Desaulniers, Tom Rix, Sami Tolvanen,
llvm, linux-kernel, patches
On Wed, Nov 02, 2022 at 09:32:51AM -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/netiucv.c:1854: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 = netiucv_tx,
> ^~~~~~~~~~
>
> ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
> 'netdev_tx_t', not 'int'. Adjust the return type of netiucv_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@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx()
2022-11-02 16:32 ` [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx() Nathan Chancellor
2022-11-02 19:09 ` Kees Cook
@ 2022-11-03 12:09 ` Alexandra Winter
1 sibling, 0 replies; 13+ messages in thread
From: Alexandra Winter @ 2022-11-03 12:09 UTC (permalink / raw)
To: Nathan Chancellor, Wenjia Zhang
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches
On 02.11.22 17:32, 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/netiucv.c:1854: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 = netiucv_tx,
> ^~~~~~~~~~
>
> ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
> 'netdev_tx_t', not 'int'. Adjust the return type of netiucv_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@kernel.org>
> ---
> drivers/s390/net/netiucv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c
> index 65aa0a96c21d..1a7f2bc3a87b 100644
> --- a/drivers/s390/net/netiucv.c
> +++ b/drivers/s390/net/netiucv.c
> @@ -1256,7 +1256,7 @@ static int netiucv_close(struct net_device *dev)
> * Note: If we return !0, then the packet is free'd by
> * the generic network layer.
> */
> -static int netiucv_tx(struct sk_buff *skb, struct net_device *dev)
> +static netdev_tx_t netiucv_tx(struct sk_buff *skb, struct net_device *dev)
> {
> struct netiucv_priv *privptr = netdev_priv(dev);
> int rc;
Could you please also remove the corresponding comments:
diff --git a/drivers/s390/net/netiucv.c b/drivers/s390/net/netiucv.c
index 65aa0a96c21d..21e36f63fbc7 100644
--- a/drivers/s390/net/netiucv.c
+++ b/drivers/s390/net/netiucv.c
@@ -1248,13 +1248,6 @@ static int netiucv_close(struct net_device *dev)
/*
* Start transmission of a packet.
* Called from generic network device layer.
- *
- * @param skb Pointer to buffer containing the packet.
- * @param dev Pointer to interface struct.
- *
- * @return 0 if packet consumed, !0 if packet rejected.
- * Note: If we return !0, then the packet is free'd by
- * the generic network layer.
*/
static int netiucv_tx(struct sk_buff *skb, struct net_device *dev)
{
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit()
2022-11-02 16:32 [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Nathan Chancellor
2022-11-02 16:32 ` [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx() Nathan Chancellor
@ 2022-11-02 16:32 ` Nathan Chancellor
2022-11-02 19:10 ` Kees Cook
2022-11-03 15:27 ` Alexandra Winter
2022-11-02 19:09 ` [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Kees Cook
2022-11-02 19:48 ` Heiko Carstens
3 siblings, 2 replies; 13+ messages in thread
From: Nathan Chancellor @ 2022-11-02 16:32 UTC (permalink / raw)
To: Alexandra Winter, Wenjia Zhang
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches, Nathan Chancellor
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/lcs.c:2090: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 = lcs_start_xmit,
^~~~~~~~~~~~~~
drivers/s390/net/lcs.c:2097: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 = lcs_start_xmit,
^~~~~~~~~~~~~~
->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
'netdev_tx_t', not 'int'. Adjust the return type of lcs_start_xmit() 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@kernel.org>
---
drivers/s390/net/lcs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
index 84c8981317b4..4cbb9802bf22 100644
--- a/drivers/s390/net/lcs.c
+++ b/drivers/s390/net/lcs.c
@@ -1519,7 +1519,7 @@ lcs_txbuffer_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
/*
* Packet transmit function called by network stack
*/
-static int
+static netdev_tx_t
__lcs_start_xmit(struct lcs_card *card, struct sk_buff *skb,
struct net_device *dev)
{
@@ -1582,7 +1582,7 @@ __lcs_start_xmit(struct lcs_card *card, struct sk_buff *skb,
return rc;
}
-static int
+static netdev_tx_t
lcs_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct lcs_card *card;
--
2.38.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit()
2022-11-02 16:32 ` [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit() Nathan Chancellor
@ 2022-11-02 19:10 ` Kees Cook
2022-11-03 15:27 ` Alexandra Winter
1 sibling, 0 replies; 13+ messages in thread
From: Kees Cook @ 2022-11-02 19:10 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexandra Winter, Wenjia Zhang, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
linux-s390, netdev, Nick Desaulniers, Tom Rix, Sami Tolvanen,
llvm, linux-kernel, patches
On Wed, Nov 02, 2022 at 09:32:52AM -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/lcs.c:2090: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 = lcs_start_xmit,
> ^~~~~~~~~~~~~~
> drivers/s390/net/lcs.c:2097: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 = lcs_start_xmit,
> ^~~~~~~~~~~~~~
>
> ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
> 'netdev_tx_t', not 'int'. Adjust the return type of lcs_start_xmit() 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@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit()
2022-11-02 16:32 ` [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit() Nathan Chancellor
2022-11-02 19:10 ` Kees Cook
@ 2022-11-03 15:27 ` Alexandra Winter
1 sibling, 0 replies; 13+ messages in thread
From: Alexandra Winter @ 2022-11-03 15:27 UTC (permalink / raw)
To: Nathan Chancellor, Wenjia Zhang
Cc: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches
On 02.11.22 17:32, 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/lcs.c:2090: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 = lcs_start_xmit,
> ^~~~~~~~~~~~~~
> drivers/s390/net/lcs.c:2097: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 = lcs_start_xmit,
> ^~~~~~~~~~~~~~
>
> ->ndo_start_xmit() in 'struct net_device_ops' expects a return type of
> 'netdev_tx_t', not 'int'. Adjust the return type of lcs_start_xmit() 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@kernel.org>
> ---
> drivers/s390/net/lcs.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
> index 84c8981317b4..4cbb9802bf22 100644
> --- a/drivers/s390/net/lcs.c
> +++ b/drivers/s390/net/lcs.c
> @@ -1519,7 +1519,7 @@ lcs_txbuffer_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
> /*
> * Packet transmit function called by network stack
> */
> -static int
> +static netdev_tx_t
> __lcs_start_xmit(struct lcs_card *card, struct sk_buff *skb,
> struct net_device *dev)
> {
> @@ -1582,7 +1582,7 @@ __lcs_start_xmit(struct lcs_card *card, struct sk_buff *skb,
> return rc;
> }
>
> -static int
> +static netdev_tx_t
> lcs_start_xmit(struct sk_buff *skb, struct net_device *dev)
> {
> struct lcs_card *card;
Thanks a lot for the fix.
Could you please also fix the indentation of these lines?
With that:
Reviewed-by: Alexandra Winter <wintera@linux.ibm.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 16:32 [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Nathan Chancellor
2022-11-02 16:32 ` [PATCH 2/3] s390/netiucv: Fix return type of netiucv_tx() Nathan Chancellor
2022-11-02 16:32 ` [PATCH 3/3] s390/lcs: Fix return type of lcs_start_xmit() Nathan Chancellor
@ 2022-11-02 19:09 ` Kees Cook
2022-11-03 11:06 ` Alexandra Winter
2022-11-02 19:48 ` Heiko Carstens
3 siblings, 1 reply; 13+ messages in thread
From: Kees Cook @ 2022-11-02 19:09 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexandra Winter, Wenjia Zhang, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
linux-s390, netdev, Nick Desaulniers, Tom Rix, Sami Tolvanen,
llvm, linux-kernel, patches
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@kernel.org>
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 19:09 ` [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Kees Cook
@ 2022-11-03 11:06 ` Alexandra Winter
0 siblings, 0 replies; 13+ messages in thread
From: Alexandra Winter @ 2022-11-03 11:06 UTC (permalink / raw)
To: Kees Cook, Nathan Chancellor
Cc: Wenjia Zhang, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Sami Tolvanen, llvm, linux-kernel,
patches
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@kernel.org>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
>
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@linux.ibm.com>
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 16:32 [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Nathan Chancellor
` (2 preceding siblings ...)
2022-11-02 19:09 ` [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx() Kees Cook
@ 2022-11-02 19:48 ` Heiko Carstens
2022-11-02 19:58 ` Nathan Chancellor
2022-11-02 20:01 ` Kees Cook
3 siblings, 2 replies; 13+ messages in thread
From: Heiko Carstens @ 2022-11-02 19:48 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Alexandra Winter, Wenjia Zhang, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches
Hi Nathan,
On Wed, Nov 02, 2022 at 09:32:50AM -0700, Nathan Chancellor wrote:
> should s390 select ARCH_SUPPORTS_CFI_CLANG in the future.
Yes, s390 should select that :)
But, is there any switch or option I need to set when compiling clang,
so it knows about the kcfi sanitizer?
I get:
clang-16: error: unsupported option '-fsanitize=kcfi' for target 's390x-ibm-linux'
> clang --version
clang version 16.0.0 (https://github.com/llvm/llvm-project.git e02110e2ab4dd71b276e887483f0e6e286d243ed)
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 19:48 ` Heiko Carstens
@ 2022-11-02 19:58 ` Nathan Chancellor
2022-11-02 20:01 ` Kees Cook
1 sibling, 0 replies; 13+ messages in thread
From: Nathan Chancellor @ 2022-11-02 19:58 UTC (permalink / raw)
To: Heiko Carstens
Cc: Alexandra Winter, Wenjia Zhang, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, Kees Cook, Sami Tolvanen, llvm,
linux-kernel, patches
Hi Heiko,
On Wed, Nov 02, 2022 at 08:48:42PM +0100, Heiko Carstens wrote:
> On Wed, Nov 02, 2022 at 09:32:50AM -0700, Nathan Chancellor wrote:
> > should s390 select ARCH_SUPPORTS_CFI_CLANG in the future.
>
> Yes, s390 should select that :)
>
> But, is there any switch or option I need to set when compiling clang,
> so it knows about the kcfi sanitizer?
>
> I get:
> clang-16: error: unsupported option '-fsanitize=kcfi' for target 's390x-ibm-linux'
>
> > clang --version
> clang version 16.0.0 (https://github.com/llvm/llvm-project.git e02110e2ab4dd71b276e887483f0e6e286d243ed)
No, kCFI is currently implemented in a target specific manner and Sami
only added AArch64 and X86 support in the initial change:
https://github.com/llvm/llvm-project/commit/cff5bef948c91e4919de8a5fb9765e0edc13f3de
He does have a generic version in progress but I assume it would not be
hard for one of your LLVM folks to add the kCFI operand bundle lowering
to the SystemZ backend to get access to it sooner (and it may allow for
a more optimized sequence of instructions if I understand correctly?):
https://reviews.llvm.org/D135411
Cheers,
Nathan
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 19:48 ` Heiko Carstens
2022-11-02 19:58 ` Nathan Chancellor
@ 2022-11-02 20:01 ` Kees Cook
2022-11-03 23:17 ` Sami Tolvanen
1 sibling, 1 reply; 13+ messages in thread
From: Kees Cook @ 2022-11-02 20:01 UTC (permalink / raw)
To: Heiko Carstens, Sami Tolvanen
Cc: Nathan Chancellor, Alexandra Winter, Wenjia Zhang, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
linux-s390, netdev, Nick Desaulniers, Tom Rix, llvm,
linux-kernel, patches
On Wed, Nov 02, 2022 at 08:48:42PM +0100, Heiko Carstens wrote:
> On Wed, Nov 02, 2022 at 09:32:50AM -0700, Nathan Chancellor wrote:
> > should s390 select ARCH_SUPPORTS_CFI_CLANG in the future.
>
> Yes, s390 should select that :)
>
> But, is there any switch or option I need to set when compiling clang,
> so it knows about the kcfi sanitizer?
>
> I get:
> clang-16: error: unsupported option '-fsanitize=kcfi' for target 's390x-ibm-linux'
>
> > clang --version
> clang version 16.0.0 (https://github.com/llvm/llvm-project.git e02110e2ab4dd71b276e887483f0e6e286d243ed)
You'll need the "generic arch support": https://reviews.llvm.org/D135411
which is _almost_ landed. Testing would be welcome, for sure!
Sami, do you have any notes on what common things were needed to get
arm64 and x86_64 booting under kCFI? My only oh-so-helpful notes are
"keep CFI away from early boot code". :P
--
Kees Cook
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 1/3] s390/ctcm: Fix return type of ctc{mp,}m_tx()
2022-11-02 20:01 ` Kees Cook
@ 2022-11-03 23:17 ` Sami Tolvanen
0 siblings, 0 replies; 13+ messages in thread
From: Sami Tolvanen @ 2022-11-03 23:17 UTC (permalink / raw)
To: Kees Cook
Cc: Heiko Carstens, Nathan Chancellor, Alexandra Winter,
Wenjia Zhang, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, linux-s390, netdev,
Nick Desaulniers, Tom Rix, llvm, linux-kernel, patches
On Wed, Nov 2, 2022 at 1:01 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Wed, Nov 02, 2022 at 08:48:42PM +0100, Heiko Carstens wrote:
> > On Wed, Nov 02, 2022 at 09:32:50AM -0700, Nathan Chancellor wrote:
> > > should s390 select ARCH_SUPPORTS_CFI_CLANG in the future.
> >
> > Yes, s390 should select that :)
> >
> > But, is there any switch or option I need to set when compiling clang,
> > so it knows about the kcfi sanitizer?
> >
> > I get:
> > clang-16: error: unsupported option '-fsanitize=kcfi' for target 's390x-ibm-linux'
> >
> > > clang --version
> > clang version 16.0.0 (https://github.com/llvm/llvm-project.git e02110e2ab4dd71b276e887483f0e6e286d243ed)
>
> You'll need the "generic arch support": https://reviews.llvm.org/D135411
> which is _almost_ landed. Testing would be welcome, for sure!
>
> Sami, do you have any notes on what common things were needed to get
> arm64 and x86_64 booting under kCFI? My only oh-so-helpful notes are
> "keep CFI away from early boot code". :P
You don't need to keep CFI away from early boot code, but bringing
this up in qemu+gdb initially is probably the best way forward. We
also had plenty of type mismatches in syscall wrappers in the
currently supported architectures, so that's another thing to watch
out for once your kernel boots far enough to start init.
Sami
^ permalink raw reply [flat|nested] 13+ messages in thread