Re: [Cluster-devel] [Ocfs2-devel] [PATCH 27/33] sctp: export sctp_setsockopt_bindx

2020-05-17 Thread David Laight
From: Matthew Wilcox
> Sent: 16 May 2020 16:37
...
> > Basically:
> >
> > This patch sequence (to be written) does the following:
> >
> > Patch 1: Change __sys_setsockopt() to allocate a kernel buffer,
> >  copy the data into it then call set_fs(KERNEL_DS).
> >  An on-stack buffer (say 64 bytes) will be used for
> >  small transfers.
> >
> > Patch 2: The same for __sys_getsockopt().
> >
> > Patch 3: Compat setsockopt.
> >
> > Patch 4: Compat getsockopt.
> >
> > Patch 5: Remove the user copies from the global socket options code.
> >
> > Patches 6 to n-1; Remove the user copies from the per-protocol code.
> >
> > Patch n: Remove the set_fs(KERNEL_DS) from the entry points.
> >
> > This should be bisectable.
> 
> I appreciate your dedication to not publishing the source code to
> your kernel module, but Christoph's patch series is actually better.
> It's typesafe rather than passing void pointers around.

There are plenty on interfaces that pass a 'pointer and length'.
Having the compiler do a type check doesn't give any security
benefit - just stops silly errors.

Oh yes, I've attached the only driver source file that calls
into the Linux kernel.
You are perfectly free to look at all the thing we have to do
to support different and broken kernel releases.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)
#ident "@(#) (c) Aculab plc $Header: 
/home/cvs/repository/ss7/stack/src/driver/linux/ss7osglue.c,v 1.157 2019-08-29 
16:09:14 davidla Exp $ $Name:  $"
#ifndef MODULE
#define MODULE
#endif

#include 

#if LINUX_VERSION_CODE < KERNEL_VERSION(2, 6, 28)
#error minimum kernel version is 2.6.28
#endif

#if LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 34)
#include 
#else
#include 
#endif

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 11, 0)
#include 
#endif
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#include 

/* This is only in the kernel build tree */
#include 

#if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 10, 0)
#include 
#else
#include /* netinet/sctp.h ought to be this file */
#endif

#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 13, 0)
#define wait_queue_head __wait_queue_head
#define wait_queue_entry __wait_queue
#endif

#define SK_PROTOCOL(sock) (sock)->sk->sk_protocol

extern void ss7_trace_mem(int, void *, int, const char *, ...);
extern void ss7_trace_printf(int, const char *, ...);

/* Aculab DACP interfaces - these are in aculab's kern_if.h */
void *dacp_symbol_get(const char *);
int dacp_symbol_release(const char *);

MODULE_AUTHOR("Aculab");
MODULE_LICENSE("Proprietary");

#include "ss7osglue.h"

/* Mutex for driver interface code */
static struct mutex ss7_glue_mutex;

static int ss7dev_major;
static const void *ss7_dtls_handle;
static int ss7_use_count;
static int ss7_stop_pid;

static struct task_struct *asserted_tasks[16];
static unsigned int asserted_task_count;

typedef char ss7_verify_const[ SS7_SOCK_STREAM == SOCK_STREAM && 
SS7_SOCK_SEQPACKET == SOCK_SEQPACKET ? 1 : -1];

static void ss7_net_ns_unload(void);

#define TCP_NODELAY 1

static int ss7_glue_open(struct inode *, struct file *);
static int ss7_glue_release(struct inode *, struct file *);
static long ss7_glue_unlocked_ioctl(struct file *, unsigned int, unsigned long);
static unsigned int ss7_glue_poll(struct file *const, poll_table *);

static struct file_operations ss7dev_fop =
{
open:   ss7_glue_open,
release:ss7_glue_release,
unlocked_ioctl: ss7_glue_unlocked_ioctl,
compat_ioctl:   ss7_glue_unlocked_ioctl,
poll:   ss7_glue_poll,
owner:  THIS_MODULE
};

static int ss7_reboot_notify(struct notifier_block *nb, unsigned long action,
void *data)
{
/* System being rebooted.
 * I added this hoping to use it to get the ss7maint daemon to exit,
 * but it isn't called until all user processes have died.
 * Leave it here - might be useful one day. */
return 0;
}

static struct notifier_block ss7_reboot_notifier_block = {
.notifier_call = ss7_reboot_notify,
};

static int
ss7_init_fail(int rval)
{
if (ss7dev_major > 0)
unregister_chrdev(ss7dev_major, "ss7server");
return rval;
}

static int
ss7_init_mod(void)
{
const void *(*dtls_register)(const char *, int (*)(struct dtls_get_if *));
int rval;

ss7_mutex_init(_glue_mutex);

printk(KERN_INFO "%s\n", ss7version);

ss7dev_major = register_chrdev(0, "ss7server", _fop);

if (ss7dev_major < 0) {
printk(KERN_INFO "ss7server: register_chrdev() failed: %d\n",
ss7dev_major);
return ss7_init_fail(ss7dev_major);
}

rval = ss7_driver_init();
if (rval != 0) {
printk(KERN_INFO "ss7server: ss7_driver_init() failed: %d\n", rval);
return ss7_init_fail(-EIO);
}

dtls_register = 

Re: [Cluster-devel] [Ocfs2-devel] [PATCH 27/33] sctp: export sctp_setsockopt_bindx

2020-05-16 Thread Matthew Wilcox
On Sat, May 16, 2020 at 03:11:40PM +, David Laight wrote:
> From: David Howells
> > Sent: 15 May 2020 16:20
> > Christoph Hellwig  wrote:
> > 
> > > > The advantage on using kernel_setsockopt here is that sctp module will
> > > > only be loaded if dlm actually creates a SCTP socket.  With this
> > > > change, sctp will be loaded on setups that may not be actually using
> > > > it. It's a quite big module and might expose the system.
> > >
> > > True.  Not that the intent is to kill kernel space callers of setsockopt,
> > > as I plan to remove the set_fs address space override used for it.
> > 
> > For getsockopt, does it make sense to have the core kernel load 
> > optval/optlen
> > into a buffer before calling the protocol driver?  Then the driver need not
> > see the userspace pointer at all.
> > 
> > Similar could be done for setsockopt - allocate a buffer of the size 
> > requested
> > by the user inside the kernel and pass it into the driver, then copy the 
> > data
> > back afterwards.
> 
> Yes, it also simplifies all the compat code.
> And there is a BPF test in setsockopt that also wants to
> pass on a kernel buffer.
> 
> I'm willing to sit and write the patch.
> Quoting from a post I made later on Friday.
> 
> Basically:
> 
> This patch sequence (to be written) does the following:
> 
> Patch 1: Change __sys_setsockopt() to allocate a kernel buffer,
>  copy the data into it then call set_fs(KERNEL_DS).
>  An on-stack buffer (say 64 bytes) will be used for
>  small transfers.
> 
> Patch 2: The same for __sys_getsockopt().
> 
> Patch 3: Compat setsockopt.
> 
> Patch 4: Compat getsockopt.
> 
> Patch 5: Remove the user copies from the global socket options code.
> 
> Patches 6 to n-1; Remove the user copies from the per-protocol code.
> 
> Patch n: Remove the set_fs(KERNEL_DS) from the entry points.
> 
> This should be bisectable.

I appreciate your dedication to not publishing the source code to
your kernel module, but Christoph's patch series is actually better.
It's typesafe rather than passing void pointers around.