lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 15 Jul 2015 09:18:28 -0400
From:	Neil Horman <nhorman@...driver.com>
To:	Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:	netdev@...r.kernel.org, Vlad Yasevich <vyasevich@...il.com>,
	linux-sctp@...r.kernel.org
Subject: Re: [PATCH v2 1/2] sctp: add new getsockopt option
 SCTP_SOCKOPT_PEELOFF_KERNEL

On Tue, Jul 14, 2015 at 02:13:24PM -0300, Marcelo Ricardo Leitner wrote:
> SCTP has this operation to peel off associations from a given socket and
> create a new socket using this association. We currently have two ways
> to use this operation:
> - via getsockopt(), on which it will also create and return a file
>   descriptor for this new socket
> - via sctp_do_peeloff(), which is for kernel only
> 
> The caveat with using sctp_do_peeloff() directly is that it creates a
> dependency to SCTP module, while all other operations are handled via
> kernel_{socket,sendmsg,getsockopt...}() interface. This causes the
> kernel to load SCTP module even when it's not really used.
> 
> This patch then creates a new sockopt that is to be used only by kernel
> users of this protocol. This new sockopt will not allocate a file
> descriptor but instead just return the socket pointer directly.
> 
> Kernel users are actually identified by if the parent socket has or not
> a fd attached to it. If not, it's a kernel a user.
> 
> If called by an user application, it will just return -EPERM.
> 
> Even though it's not intended for user applications, it's listed under
> uapi header. That's because hidding this wouldn't add any extra security
> and to keep the sockopt list in one place, so it's easy to check
> available numbers to use.
> 
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> ---
>  include/uapi/linux/sctp.h | 12 ++++++++++++
>  net/sctp/socket.c         | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+)
> 
> diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
> index ce70fe6b45df3e841c35accbdb6379c16563893c..b3aad3ce456ab3c1ebf4d81fdb7269ba40b3d92a 100644
> --- a/include/uapi/linux/sctp.h
> +++ b/include/uapi/linux/sctp.h
> @@ -105,6 +105,10 @@ typedef __s32 sctp_assoc_t;
>  #define SCTP_SOCKOPT_BINDX_ADD	100	/* BINDX requests for adding addrs */
>  #define SCTP_SOCKOPT_BINDX_REM	101	/* BINDX requests for removing addrs. */
>  #define SCTP_SOCKOPT_PEELOFF	102	/* peel off association. */
> +#define SCTP_SOCKOPT_PEELOFF_KERNEL	103	/* peel off association.
> +						 * only valid for kernel
> +						 * users
> +						 */
>  /* Options 104-106 are deprecated and removed. Do not use this space */
>  #define SCTP_SOCKOPT_CONNECTX_OLD	107	/* CONNECTX old requests. */
>  #define SCTP_GET_PEER_ADDRS	108		/* Get all peer address. */
> @@ -892,6 +896,14 @@ typedef struct {
>  	int sd;
>  } sctp_peeloff_arg_t;
>  
> +/* This is the union that is passed as an argument(optval) to
> + * getsockopt(SCTP_SOCKOPT_PEELOFF_KERNEL).
> + */
> +typedef union {
> +	sctp_assoc_t associd;
> +	struct socket *socket;
> +} sctp_peeloff_kernel_arg_t;
> +
>  /*
>   *  Peer Address Thresholds socket option
>   */
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index f1a65398f3118ab5d3a884e9c875620560e6b5ef..7968de7a1aeabd5cd0a0398461dbf2081bd4c5b7 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4504,6 +4504,39 @@ out:
>  	return retval;
>  }
>  
> +static int sctp_getsockopt_peeloff_kernel(struct sock *sk, int len,
> +					  char __user *optval, int __user *optlen)
> +{
> +	sctp_peeloff_kernel_arg_t peeloff;
> +	struct socket *newsock;
> +	int retval = 0;
> +
> +	/* We only allow this operation if parent socket also hadn't a
> +	 * file descriptor allocated to it, mainly as a way to make sure
> +	 * that this is really a kernel socket.
> +	 */
> +	if (sk->sk_socket->file)
> +		return -EPERM;
> +
> +	if (len < sizeof(sctp_peeloff_kernel_arg_t))
> +		return -EINVAL;
> +	len = sizeof(sctp_peeloff_kernel_arg_t);
> +	if (copy_from_user(&peeloff, optval, len))
> +		return -EFAULT;
> +
> +	retval = sctp_do_peeloff(sk, peeloff.associd, &newsock);
> +	if (retval < 0)
> +		goto out;
> +
> +	peeloff.socket = newsock;
> +	if (copy_to_user(optval, &peeloff, len)) {
> +		sock_release(newsock);
> +		return -EFAULT;
> +	}
> +out:
> +	return retval;
> +}
> +
>  /* 7.1.13 Peer Address Parameters (SCTP_PEER_ADDR_PARAMS)
>   *
>   * Applications can enable or disable heartbeats for any peer address of
> @@ -5991,6 +6024,10 @@ static int sctp_getsockopt(struct sock *sk, int level, int optname,
>  	case SCTP_SOCKOPT_PEELOFF:
>  		retval = sctp_getsockopt_peeloff(sk, len, optval, optlen);
>  		break;
> +	case SCTP_SOCKOPT_PEELOFF_KERNEL:
> +		retval = sctp_getsockopt_peeloff_kernel(sk, len, optval,
> +							optlen);
> +		break;
Do we need to do something here to prevent user space calls from inadvertently
accessing this option?

Neil

>  	case SCTP_PEER_ADDR_PARAMS:
>  		retval = sctp_getsockopt_peer_addr_params(sk, len, optval,
>  							  optlen);
> -- 
> 2.4.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ