uapi header mismatch with kernel ?

uapi header mismatch with kernel ?

[苏庆]

Hi all,

I found that In 3.x kernel, the sctp states are defined differently
between kernel and uapi header. This makes our application fails to
work.

Is that a mismatch or am I missing something?

----------------------------------------------------------------
in include/uapi/linux/sctp.h

/* Association states.  */
enum sctp_sstat_state {
    SCTP_EMPTY                = 0,
    SCTP_CLOSED               = 1,
    SCTP_COOKIE_WAIT          = 2,
    SCTP_COOKIE_ECHOED        = 3,
    SCTP_ESTABLISHED          = 4,
    SCTP_SHUTDOWN_PENDING     = 5,
    SCTP_SHUTDOWN_SENT        = 6,
    SCTP_SHUTDOWN_RECEIVED    = 7,
    SCTP_SHUTDOWN_ACK_SENT    = 8,
};

in include/net/sctp/constants.h

/* SCTP state defines for internal state machine */
typedef enum {

        SCTP_STATE_CLOSED               = 0,
        SCTP_STATE_COOKIE_WAIT          = 1,
        SCTP_STATE_COOKIE_ECHOED        = 2,
        SCTP_STATE_ESTABLISHED          = 3,
        SCTP_STATE_SHUTDOWN_PENDING     = 4,
        SCTP_STATE_SHUTDOWN_SENT        = 5,
        SCTP_STATE_SHUTDOWN_RECEIVED    = 6,
        SCTP_STATE_SHUTDOWN_ACK_SENT    = 7,

} sctp_state_t;



and it looks like the SCTP_STATE_EMPTY was removed by this
patch:http://lists.openwall.net/netdev/2011/04/20/31

Any help will be appreciated.

Thanks,
Tristan

Re: uapi header mismatch with kernel ?

[Daniel Borkmann]

On 08/28/2014 08:33 AM, 苏庆 wrote:
> Hi all,
>
> I found that In 3.x kernel, the sctp states are defined differently
> between kernel and uapi header. This makes our application fails to
> work.
>
> Is that a mismatch or am I missing something?

You are correct, and that's not good ... can you try out the below
kernel patch?

 From 90653829a4d406898f6906849f3eca481ec01894 Mon Sep 17 00:00:00 2001
From: Daniel Borkmann <dborkman@redhat.com>
Date: Thu, 28 Aug 2014 09:26:36 +0200
Subject: [PATCH] net: sctp: fix ABI through sctp_assoc_to_state helper

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
  include/net/sctp/sctp.h | 13 +++++++++++++
  net/sctp/socket.c       |  2 +-
  2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index f6e7397..f50dccf 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -320,6 +320,19 @@ static inline sctp_assoc_t sctp_assoc2id(const struct sctp_association *asoc)
  	return asoc ? asoc->assoc_id : 0;
  }

+static inline enum sctp_sstat_state
+sctp_assoc_to_state(const struct sctp_association *asoc)
+{
+	/* SCTP's uapi always had SCTP_EMPTY(=0) as a dummy state, but we
+	 * got rid of it in kernel space. Therefore SCTP_CLOSED et al
+	 * start at =1 in user space, but actually as =0 in kernel space.
+	 * Now that we may not break user space and SCTP_EMPTY is exposed
+	 * there, hence we need to fix it up with an ugly offset to not
+	 * break applications. :(
+	 */
+	return asoc->state + 1;
+}
+
  /* Look up the association by its id.  */
  struct sctp_association *sctp_id2assoc(struct sock *sk, sctp_assoc_t id);

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index eb71d49..634a2ab 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4243,7 +4243,7 @@ static int sctp_getsockopt_sctp_status(struct sock *sk, int len,
  	transport = asoc->peer.primary_path;

  	status.sstat_assoc_id = sctp_assoc2id(asoc);
-	status.sstat_state = asoc->state;
+	status.sstat_state = sctp_assoc_to_state(asoc);
  	status.sstat_rwnd =  asoc->peer.rwnd;
  	status.sstat_unackdata = asoc->unack_data;

-- 
1.7.11.7

Re: uapi header mismatch with kernel ?

[Daniel Borkmann]

On 08/28/2014 09:34 AM, Daniel Borkmann wrote:
> On 08/28/2014 08:33 AM, 苏庆 wrote:
>> Hi all,
>>
>> I found that In 3.x kernel, the sctp states are defined differently
>> between kernel and uapi header. This makes our application fails to
>> work.
>>
>> Is that a mismatch or am I missing something?
>
> You are correct, and that's not good ... can you try out the below
> kernel patch?

Note, the problem is that we cannot just redefine the sctp_state_t
enum back to as it was as it's accessing a function pointer lookup
table, so I think it's better to just add this offset for user space
now. It's read-only anyway.

>  From 90653829a4d406898f6906849f3eca481ec01894 Mon Sep 17 00:00:00 2001
> From: Daniel Borkmann <dborkman@redhat.com>
> Date: Thu, 28 Aug 2014 09:26:36 +0200
> Subject: [PATCH] net: sctp: fix ABI through sctp_assoc_to_state helper
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
>   include/net/sctp/sctp.h | 13 +++++++++++++
>   net/sctp/socket.c       |  2 +-
>   2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index f6e7397..f50dccf 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -320,6 +320,19 @@ static inline sctp_assoc_t sctp_assoc2id(const struct sctp_association *asoc)
>       return asoc ? asoc->assoc_id : 0;
>   }
>
> +static inline enum sctp_sstat_state
> +sctp_assoc_to_state(const struct sctp_association *asoc)
> +{
> +    /* SCTP's uapi always had SCTP_EMPTY(=0) as a dummy state, but we
> +     * got rid of it in kernel space. Therefore SCTP_CLOSED et al
> +     * start at =1 in user space, but actually as =0 in kernel space.
> +     * Now that we may not break user space and SCTP_EMPTY is exposed
> +     * there, hence we need to fix it up with an ugly offset to not
> +     * break applications. :(
> +     */
> +    return asoc->state + 1;
> +}
> +
>   /* Look up the association by its id.  */
>   struct sctp_association *sctp_id2assoc(struct sock *sk, sctp_assoc_t id);
>
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index eb71d49..634a2ab 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4243,7 +4243,7 @@ static int sctp_getsockopt_sctp_status(struct sock *sk, int len,
>       transport = asoc->peer.primary_path;
>
>       status.sstat_assoc_id = sctp_assoc2id(asoc);
> -    status.sstat_state = asoc->state;
> +    status.sstat_state = sctp_assoc_to_state(asoc);
>       status.sstat_rwnd =  asoc->peer.rwnd;
>       status.sstat_unackdata = asoc->unack_data;
>

Re: uapi header mismatch with kernel ?

[Tristan Su]

On Thu, Aug 28, 2014 at 3:47 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 08/28/2014 09:34 AM, Daniel Borkmann wrote:
>>
>> On 08/28/2014 08:33 AM, 苏庆 wrote:
>>>
>>> Hi all,
>>>
>>> I found that In 3.x kernel, the sctp states are defined differently
>>> between kernel and uapi header. This makes our application fails to
>>> work.
>>>
>>> Is that a mismatch or am I missing something?
>>
>>
>> You are correct, and that's not good ... can you try out the below
>> kernel patch?

Thank you for the confirmation.
I'm not kernel developer and have no experience on kernel patching/compiling
but the fix looks straightforwards to me.

It seems that the issue exists in mainline kernel and also distributions
 like RHEL7, Ubuntu 14.04, etc. Applications that care SCTP_STATUS
 would be impacted and it would not help even to recompile the application.

BTW, any idea on how long it would take for the fix to be delivered by
distributions like RHEL?

Should I report this to bugzilla.kernel.org or to linux distributions?

Thanks again!
-- 
Tristan

Re: uapi header mismatch with kernel ?

[Daniel Borkmann]

On 08/28/2014 03:05 PM, Tristan Su wrote:
> On Thu, Aug 28, 2014 at 3:47 PM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> On 08/28/2014 09:34 AM, Daniel Borkmann wrote:
>>>
>>> On 08/28/2014 08:33 AM, 苏庆 wrote:
>>>>
>>>> Hi all,
>>>>
>>>> I found that In 3.x kernel, the sctp states are defined differently
>>>> between kernel and uapi header. This makes our application fails to
>>>> work.
>>>>
>>>> Is that a mismatch or am I missing something?
>>>
>>>
>>> You are correct, and that's not good ... can you try out the below
>>> kernel patch?
>
> Thank you for the confirmation.
> I'm not kernel developer and have no experience on kernel patching/compiling
> but the fix looks straightforwards to me.
>
> It seems that the issue exists in mainline kernel and also distributions
>   like RHEL7, Ubuntu 14.04, etc. Applications that care SCTP_STATUS
>   would be impacted and it would not help even to recompile the application.

I'm currently verifying it myself and will send out a patch ASAP. It will
then go to stable afterwards on the next pull request from Dave to Linus.

> BTW, any idea on how long it would take for the fix to be delivered by
> distributions like RHEL?
>
> Should I report this to bugzilla.kernel.org or to linux distributions?

They will include it when it gets delivered via stable kernels.