[lkml]   [2018]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH v2] ppp: remove the PPPIOCDETACH ioctl
On Wed, May 23, 2018 at 02:37:38PM -0700, Eric Biggers wrote:
> From: Eric Biggers <>
> The PPPIOCDETACH ioctl effectively tries to "close" the given ppp file
> before f_count has reached 0, which is fundamentally a bad idea. It
> does check 'f_count < 2', which excludes concurrent operations on the
> file since they would only be possible with a shared fd table, in which
> case each fdget() would take a file reference. However, it fails to
> account for the fact that even with 'f_count == 1' the file can still be
> linked into epoll instances. As reported by syzbot, this can trivially
> be used to cause a use-after-free.
> Yet, the only known user of PPPIOCDETACH is pppd versions older than
> ppp-2.4.2, which was released almost 15 years ago (November 2003).
> Also, PPPIOCDETACH apparently stopped working reliably at around the
> same time, when the f_count check was added to the kernel, e.g. see
> Also, the current 'f_count < 2'
> check makes PPPIOCDETACH only work in single-threaded applications; it
> always fails if called from a multithreaded application.
> All pppd versions released in the last 15 years just close() the file
> descriptor instead.
> Therefore, instead of hacking around this bug by exporting epoll
> internals to modules, and probably missing other related bugs, just
> remove the PPPIOCDETACH ioctl and see if anyone actually notices. Leave
> a stub in place that prints a one-time warning and returns EINVAL.
> Reported-by:
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc:
> Signed-off-by: Eric Biggers <>
> ---
> v2: leave a stub in place, rather than removing the ioctl completely.
Thanks a lot for your help on this matter.

BTW, netdev has its own rules wrt. stable backports. You didn't need to
CC: stable@. David handles -stable submissions himself.
Using a 'PATCH net' subject prefix would have made it clear that this
patch was fixing some released code and should be considered for -stable

Reviewed-by: Guillaume Nault <>
Tested-by: Guillaume Nault <>

 \ /
  Last update: 2018-05-24 16:06    [W:0.068 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site