linux-kernel - Re: [PATCH] ptmx: adding handshake support

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:	Sun, 13 Apr 2008 23:35:03 +0100
From:	Alan Cox <alan@...rguk.ukuu.org.uk>
To:	sander@...-ginkel.eu
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ptmx: adding handshake support

> + *  Added support for MCR/MSR, used for serial over ethernet
> + *    -- Sander van Ginkel <sander@...-ginkel.eu>

We've been trying to get rid of these long lists in the code and put them
in the git tree (git whatchanged/git blame show the info rather better)

> +
> +    /* initialize the pointer in case something fails */
> +   
> +    tty->driver_data = NULL;                         
> +    tty->link->driver_data = NULL;

Not needed

  +    /* first time accessing this device, let's create it */
> +
> +    mcrmsr = kmalloc(sizeof(*mcrmsr), GFP_KERNEL);

kzalloc will clear it for you...

> +static int pty_tiocmset(struct tty_struct *tty, struct file 
> *file,unsigned int set, unsigned int clear)
> +{
> +    unsigned int *mcrmsr;
> +
> +    mcrmsr = tty->driver_data;
> +    *mcrmsr=set;
> +    tty->driver_data=mcrmsr;

Why this last assignment ?

> +
> +    case VMCRMSR: /* Set all of the handshake line, even the normally 
> read only */
> +        {
> +        if (copy_from_user(&value,(unsigned int *)arg,sizeof(unsigned 
> int)))
> +            return -EFAULT;
> +
> +        *mcrmsr=value;

Ok - possibly we shouldn't allow people to set undefined bits but I'm not
sure it matters

> +        tty->driver_data=mcrmsr;

Why ??


I am curious how this is handled by other Unix systems and if there is an
ioctl we can follow from other systems ?


Looks basically ok, coding style is wrong, some odd extra assignments but
I agree entirely with the idea of adding this functionality to keep
remote serial drivers in user space.

I'll try and find out if other Unixes have similar features we can use to
keep API consistency tidy it up and fold it at some point in the next
couple of weeks.

Alan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/