linux-kernel - Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in dgnc_tty

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:	Sat, 11 Apr 2015 15:20:43 +0300 (EEST)
From:	Giedrius Statkevičius 
	<giedrius.statkevicius@...il.com>
To:	Sudip Mukherjee <sudipm.mukherjee@...il.com>
cc:	Giedrius Statkevičius 
	<giedrius.statkevicius@...il.com>, lidza.louina@...il.com,
	markh@...pro.net, gregkh@...uxfoundation.org,
	driverdev-devel@...uxdriverproject.org, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2 RESEND] staging: dgnc: remove dead code in
 dgnc_tty_write()

On Sat, 11 Apr 2015, Sudip Mukherjee wrote:

> On Fri, Apr 10, 2015 at 05:48:54PM +0300, Giedrius Statkevičius wrote:
> > Remove the dead code protected by in_user in dgnc_tty_write() because it is set
> > to 0 and never changed to 1 thus the code in ifs never gets executed.
> dgnc_tty_write() is being called by dgnc_tty_put_char() and it is also
> the write callback function of struct tty_operations, so I think the
> correct fix will be to use from_user and make it 0 when
> dgnc_tty_put_char() calls this function else make it 1 to inform the
> function that the data wil be coming from the userspace. Maybe some
> thing like this:
Well, I think this is wrong because:
* parameter of write member of struct tty_operations buf is not tagged with
  "__user" so it should be safe to just memcpy() from it
* Looked through some other write operations in other tty drivers and I've never
  seen copy_from_user() used on buf argument of write operation - always
  memcpy() or some other similar function
* Ldd3 (and the comments in tty_driver.h) says that write could be called from
  interrupt context too so it can't sleep and thus use copy_from_user()

While looking through I've caught that dgnc_TmpWriteBuf could be also deleted
because it will be unused if this patch goes through. If it does then I'll send
another one (or should I send a v3?)

Su pagarba / Regards,
Giedrius