Re: [PATCH V8 2/4] sctp: Add ip option support

2018-02-27 Thread Neil Horman 
On Mon, Feb 26, 2018 at 05:48:48PM -0500, Paul Moore wrote:
> On Sat, Feb 24, 2018 at 11:18 AM, Richard Haines
>  wrote:
> > Add ip option support to allow LSM security modules to utilise CIPSO/IPv4
> > and CALIPSO/IPv6 services.
> >
> > Signed-off-by: Richard Haines 
> > ---
> > All SCTP lksctp-tools/src/func_tests run correctly in enforcing mode.
> > All "./sctp-tests run" obtained from: https://github.com/sctp/sctp-tests
> > pass.
> >
> > V7 Changes:
> > 1) Log when copy ip options fail for IPv4 and IPv6
> > 2) Correct sctp_setsockopt_maxseg() function. Note that the lksctp-tools
> > func_tests do not test with struct sctp_assoc_value. Just used simple test
> > and okay.
> > 3) Move calculation of overheads to sctp_packet_config().
> > NOTE: Initially in sctp_packet_reset() I set packet->size and
> > packet->overhead to zero (as it is a reset). This was okay for all the
> > lksctp-tools function tests, however when running "sctp-tests" ndatshched
> > tests it causes these to fail with an st_s.log entry of:
> > sid: 3, expected: 3
> > sid: 3, expected: 3
> > unexpected sid packet !!!
> > sid: 1, expected: 3
> >
> > I then found sctp_packet_transmit() relies on setting
> > "packet->size = packet->overhead;" to reset size to the current overhead
> > after sending packets, hence the comment in sctp_packet_reset()
> >
> > V8 Change:
> > Fix sparse warning:
> > net/sctp/protocol.c:269:28: sparse: dereference of noderef expression
> > highlighted in [1] for sctp_v4_ip_options_len() function.
> >
> > [1] https://lists.01.org/pipermail/kbuild-all/2018-February/043695.html
> >
> >  include/net/sctp/sctp.h|  4 +++-
> >  include/net/sctp/structs.h |  2 ++
> >  net/sctp/chunk.c   | 10 +++---
> >  net/sctp/ipv6.c| 45 
> > ++---
> >  net/sctp/output.c  | 34 +-
> >  net/sctp/protocol.c| 43 +++
> >  net/sctp/socket.c  | 11 ---
> >  7 files changed, 122 insertions(+), 27 deletions(-)
> 
> Thanks Richard.
> 
> Neil and Marcelo, I transfered your acked-by to this patch, if you've
> got any objections to that please let me know.
> 
I'm also fine with the transfer, thanks for checking!
Neil

Re: [PATCH V8 2/4] sctp: Add ip option support

2018-02-27 Thread Marcelo Ricardo Leitner 
On Mon, Feb 26, 2018 at 05:48:48PM -0500, Paul Moore wrote:
> On Sat, Feb 24, 2018 at 11:18 AM, Richard Haines
>  wrote:
> > Add ip option support to allow LSM security modules to utilise CIPSO/IPv4
> > and CALIPSO/IPv6 services.
> >
> > Signed-off-by: Richard Haines 
> > ---
> > All SCTP lksctp-tools/src/func_tests run correctly in enforcing mode.
> > All "./sctp-tests run" obtained from: https://github.com/sctp/sctp-tests
> > pass.
> >
> > V7 Changes:
> > 1) Log when copy ip options fail for IPv4 and IPv6
> > 2) Correct sctp_setsockopt_maxseg() function. Note that the lksctp-tools
> > func_tests do not test with struct sctp_assoc_value. Just used simple test
> > and okay.
> > 3) Move calculation of overheads to sctp_packet_config().
> > NOTE: Initially in sctp_packet_reset() I set packet->size and
> > packet->overhead to zero (as it is a reset). This was okay for all the
> > lksctp-tools function tests, however when running "sctp-tests" ndatshched
> > tests it causes these to fail with an st_s.log entry of:
> > sid: 3, expected: 3
> > sid: 3, expected: 3
> > unexpected sid packet !!!
> > sid: 1, expected: 3
> >
> > I then found sctp_packet_transmit() relies on setting
> > "packet->size = packet->overhead;" to reset size to the current overhead
> > after sending packets, hence the comment in sctp_packet_reset()
> >
> > V8 Change:
> > Fix sparse warning:
> > net/sctp/protocol.c:269:28: sparse: dereference of noderef expression
> > highlighted in [1] for sctp_v4_ip_options_len() function.
> >
> > [1] https://lists.01.org/pipermail/kbuild-all/2018-February/043695.html
> >
> >  include/net/sctp/sctp.h|  4 +++-
> >  include/net/sctp/structs.h |  2 ++
> >  net/sctp/chunk.c   | 10 +++---
> >  net/sctp/ipv6.c| 45 
> > ++---
> >  net/sctp/output.c  | 34 +-
> >  net/sctp/protocol.c| 43 +++
> >  net/sctp/socket.c  | 11 ---
> >  7 files changed, 122 insertions(+), 27 deletions(-)
> 
> Thanks Richard.
> 
> Neil and Marcelo, I transfered your acked-by to this patch, if you've
> got any objections to that please let me know.

That's fine by me. Thanks

  Marcelo

Re: [PATCH V8 2/4] sctp: Add ip option support

2018-02-27 Thread Paul Moore 
On Sat, Feb 24, 2018 at 11:18 AM, Richard Haines
 wrote:
> Add ip option support to allow LSM security modules to utilise CIPSO/IPv4
> and CALIPSO/IPv6 services.
>
> Signed-off-by: Richard Haines 
> ---
> All SCTP lksctp-tools/src/func_tests run correctly in enforcing mode.
> All "./sctp-tests run" obtained from: https://github.com/sctp/sctp-tests
> pass.
>
> V7 Changes:
> 1) Log when copy ip options fail for IPv4 and IPv6
> 2) Correct sctp_setsockopt_maxseg() function. Note that the lksctp-tools
> func_tests do not test with struct sctp_assoc_value. Just used simple test
> and okay.
> 3) Move calculation of overheads to sctp_packet_config().
> NOTE: Initially in sctp_packet_reset() I set packet->size and
> packet->overhead to zero (as it is a reset). This was okay for all the
> lksctp-tools function tests, however when running "sctp-tests" ndatshched
> tests it causes these to fail with an st_s.log entry of:
> sid: 3, expected: 3
> sid: 3, expected: 3
> unexpected sid packet !!!
> sid: 1, expected: 3
>
> I then found sctp_packet_transmit() relies on setting
> "packet->size = packet->overhead;" to reset size to the current overhead
> after sending packets, hence the comment in sctp_packet_reset()
>
> V8 Change:
> Fix sparse warning:
> net/sctp/protocol.c:269:28: sparse: dereference of noderef expression
> highlighted in [1] for sctp_v4_ip_options_len() function.
>
> [1] https://lists.01.org/pipermail/kbuild-all/2018-February/043695.html
>
>  include/net/sctp/sctp.h|  4 +++-
>  include/net/sctp/structs.h |  2 ++
>  net/sctp/chunk.c   | 10 +++---
>  net/sctp/ipv6.c| 45 ++---
>  net/sctp/output.c  | 34 +-
>  net/sctp/protocol.c| 43 +++
>  net/sctp/socket.c  | 11 ---
>  7 files changed, 122 insertions(+), 27 deletions(-)

Thanks Richard.

Neil and Marcelo, I transfered your acked-by to this patch, if you've
got any objections to that please let me know.

> diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
> index f7ae6b0..25c5c87 100644
> --- a/include/net/sctp/sctp.h
> +++ b/include/net/sctp/sctp.h
> @@ -441,9 +441,11 @@ static inline int sctp_list_single_entry(struct 
> list_head *head)
>  static inline int sctp_frag_point(const struct sctp_association *asoc, int 
> pmtu)
>  {
> struct sctp_sock *sp = sctp_sk(asoc->base.sk);
> +   struct sctp_af *af = sp->pf->af;
> int frag = pmtu;
>
> -   frag -= sp->pf->af->net_header_len;
> +   frag -= af->ip_options_len(asoc->base.sk);
> +   frag -= af->net_header_len;
> frag -= sizeof(struct sctphdr) + sctp_datachk_len(>stream);
>
> if (asoc->user_frag)
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 03e92dd..ead5fce 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -491,6 +491,7 @@ struct sctp_af {
> void(*ecn_capable)(struct sock *sk);
> __u16   net_header_len;
> int sockaddr_len;
> +   int (*ip_options_len)(struct sock *sk);
> sa_family_t sa_family;
> struct list_head list;
>  };
> @@ -515,6 +516,7 @@ struct sctp_pf {
> int (*addr_to_user)(struct sctp_sock *sk, union sctp_addr *addr);
> void (*to_sk_saddr)(union sctp_addr *, struct sock *sk);
> void (*to_sk_daddr)(union sctp_addr *, struct sock *sk);
> +   void (*copy_ip_options)(struct sock *sk, struct sock *newsk);
> struct sctp_af *af;
>  };
>
> diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
> index 991a530..d726d21 100644
> --- a/net/sctp/chunk.c
> +++ b/net/sctp/chunk.c
> @@ -171,6 +171,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct 
> sctp_association *asoc,
> struct list_head *pos, *temp;
> struct sctp_chunk *chunk;
> struct sctp_datamsg *msg;
> +   struct sctp_sock *sp;
> +   struct sctp_af *af;
> int err;
>
> msg = sctp_datamsg_new(GFP_KERNEL);
> @@ -189,9 +191,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct 
> sctp_association *asoc,
> /* This is the biggest possible DATA chunk that can fit into
>  * the packet
>  */
> -   max_data = asoc->pathmtu -
> -  sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> -  sizeof(struct sctphdr) - sctp_datachk_len(>stream);
> +   sp = sctp_sk(asoc->base.sk);
> +   af = sp->pf->af;
> +   max_data = asoc->pathmtu - af->net_header_len -
> +  sizeof(struct sctphdr) - sctp_datachk_len(>stream) -
> +  af->ip_options_len(asoc->base.sk);
> max_data = SCTP_TRUNC4(max_data);
>
> /* If the the peer requested that we authenticate DATA chunks
> diff --git a/net/sctp/ipv6.c

[PATCH V8 2/4] sctp: Add ip option support

2018-02-26 Thread Richard Haines via Selinux 
Add ip option support to allow LSM security modules to utilise CIPSO/IPv4
and CALIPSO/IPv6 services.

Signed-off-by: Richard Haines 
---
All SCTP lksctp-tools/src/func_tests run correctly in enforcing mode.
All "./sctp-tests run" obtained from: https://github.com/sctp/sctp-tests
pass.

V7 Changes:
1) Log when copy ip options fail for IPv4 and IPv6
2) Correct sctp_setsockopt_maxseg() function. Note that the lksctp-tools
func_tests do not test with struct sctp_assoc_value. Just used simple test
and okay.
3) Move calculation of overheads to sctp_packet_config().
NOTE: Initially in sctp_packet_reset() I set packet->size and
packet->overhead to zero (as it is a reset). This was okay for all the
lksctp-tools function tests, however when running "sctp-tests" ndatshched
tests it causes these to fail with an st_s.log entry of:
sid: 3, expected: 3
sid: 3, expected: 3
unexpected sid packet !!!
sid: 1, expected: 3

I then found sctp_packet_transmit() relies on setting
"packet->size = packet->overhead;" to reset size to the current overhead
after sending packets, hence the comment in sctp_packet_reset()

V8 Change:
Fix sparse warning:
net/sctp/protocol.c:269:28: sparse: dereference of noderef expression
highlighted in [1] for sctp_v4_ip_options_len() function.

[1] https://lists.01.org/pipermail/kbuild-all/2018-February/043695.html

 include/net/sctp/sctp.h|  4 +++-
 include/net/sctp/structs.h |  2 ++
 net/sctp/chunk.c   | 10 +++---
 net/sctp/ipv6.c| 45 ++---
 net/sctp/output.c  | 34 +-
 net/sctp/protocol.c| 43 +++
 net/sctp/socket.c  | 11 ---
 7 files changed, 122 insertions(+), 27 deletions(-)

diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index f7ae6b0..25c5c87 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -441,9 +441,11 @@ static inline int sctp_list_single_entry(struct list_head 
*head)
 static inline int sctp_frag_point(const struct sctp_association *asoc, int 
pmtu)
 {
struct sctp_sock *sp = sctp_sk(asoc->base.sk);
+   struct sctp_af *af = sp->pf->af;
int frag = pmtu;
 
-   frag -= sp->pf->af->net_header_len;
+   frag -= af->ip_options_len(asoc->base.sk);
+   frag -= af->net_header_len;
frag -= sizeof(struct sctphdr) + sctp_datachk_len(>stream);
 
if (asoc->user_frag)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 03e92dd..ead5fce 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -491,6 +491,7 @@ struct sctp_af {
void(*ecn_capable)(struct sock *sk);
__u16   net_header_len;
int sockaddr_len;
+   int (*ip_options_len)(struct sock *sk);
sa_family_t sa_family;
struct list_head list;
 };
@@ -515,6 +516,7 @@ struct sctp_pf {
int (*addr_to_user)(struct sctp_sock *sk, union sctp_addr *addr);
void (*to_sk_saddr)(union sctp_addr *, struct sock *sk);
void (*to_sk_daddr)(union sctp_addr *, struct sock *sk);
+   void (*copy_ip_options)(struct sock *sk, struct sock *newsk);
struct sctp_af *af;
 };
 
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 991a530..d726d21 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -171,6 +171,8 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct 
sctp_association *asoc,
struct list_head *pos, *temp;
struct sctp_chunk *chunk;
struct sctp_datamsg *msg;
+   struct sctp_sock *sp;
+   struct sctp_af *af;
int err;
 
msg = sctp_datamsg_new(GFP_KERNEL);
@@ -189,9 +191,11 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct 
sctp_association *asoc,
/* This is the biggest possible DATA chunk that can fit into
 * the packet
 */
-   max_data = asoc->pathmtu -
-  sctp_sk(asoc->base.sk)->pf->af->net_header_len -
-  sizeof(struct sctphdr) - sctp_datachk_len(>stream);
+   sp = sctp_sk(asoc->base.sk);
+   af = sp->pf->af;
+   max_data = asoc->pathmtu - af->net_header_len -
+  sizeof(struct sctphdr) - sctp_datachk_len(>stream) -
+  af->ip_options_len(asoc->base.sk);
max_data = SCTP_TRUNC4(max_data);
 
/* If the the peer requested that we authenticate DATA chunks
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index e35d4f7..30a05a8 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -427,6 +427,41 @@ static void sctp_v6_copy_addrlist(struct list_head 
*addrlist,
rcu_read_unlock();
 }
 
+/* Copy over any ip options */
+static void sctp_v6_copy_ip_options(struct sock *sk, struct sock *newsk)
+{
+   struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
+   struct ipv6_txoptions *opt;
+
+   newnp = inet6_sk(newsk);
+
+