On Thu, May 28, 2020 at 12:26:39PM +0200, Martin Pieuchot wrote:
> On 27/05/20(Wed) 11:54, Vitaliy Makkoveev wrote:
> > pipex(4) is simultaneously protected by KERNEL_LOCK() and NET_LOCK() and
> > the last is not required. I guess to start remove NET_LOCK(). Diff below
>
On Thu, May 28, 2020 at 12:14:43PM +0200, Martin Pieuchot wrote:
> On 26/05/20(Tue) 16:12, Vitaliy Makkoveev wrote:
> > `pppx_if' has `pxi_ready' field used to prevent access to incomplete
> > `pxi'. But we don't prevent access to `pxi' which we destroy.
> > pppx_if_destroy(
socket(2) layer is already protected by solock(). It grabs NET_LOCK()
for inet{,6}(4) sockets, but all other sockets are still under
KERNEL_LOCK().
I guess solock is already placed everythere it's required. Also `struct
file' is already mp-safe.
Diff below introduces rwlock `unp_lock'. It's
pipex(4) is simultaneously protected by KERNEL_LOCK() and NET_LOCK() and
the last is not required. I guess to start remove NET_LOCK(). Diff below
drops NET_LOCK() in pipex_ioctl() and underlaying routines. At least
this helps to kill unlock/lock mess in pppx_add_session() and
pppx_if_destroy().
> On 27 May 2020, at 01:29, Sergey Ryazanov wrote:
>
> On Tue, May 26, 2020 at 12:07 PM Vitaliy Makkoveev
> wrote:
>>> On 25 May 2020, at 22:04, Sergey Ryazanov wrote:
>>> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
>>> wrote:
>>>>
`pppx_if' has `pxi_ready' field used to prevent access to incomplete
`pxi'. But we don't prevent access to `pxi' which we destroy.
pppx_if_destroy() can sleep so we can grab `pxi' which we already
destroying by concurrent thread and cause use-after-free issue. I guess
to use `pxi_ready' to prevent
> On 25 May 2020, at 22:04, Sergey Ryazanov wrote:
>
> Hello Vitaliy,
>
> On Sat, May 23, 2020 at 3:07 PM Vitaliy Makkoveev
> wrote:
>>> On 23 May 2020, at 13:11, Sergey Ryazanov wrote:
>>> On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev
>>&g
> On 26 May 2020, at 11:31, Claudio Jeker wrote:
>
> [skip]
>
> Is pppd(8) still using K function declarations? Can we please add new
> functions with ANSI declarations instead and convert the rest as well.
> Also it looks like something strange is going on with indentation (just
> look at
This time it's not clean which lock is really required to protect
pipex(4) data structures. In fact KERNL_LOCK() and NET_LOCK() are
used simultaneously. It's time to document it.
Only [k] used do mark mutable members due to NET_LOCK() looks be
unnecesary and can be killed in future.
Index:
`pppx_ifs_lk' used to protect `pppx_ifs' tree, But this tree is accessed
under KERNL_LOCK() and there is no concurency. Also we don't sleep while
holding this lock. So it's useless, kill it for simplification.
Index: sys/net/if_pppx.c
Sorry about delay.
> On 20 May 2020, at 10:54, Martin Pieuchot wrote:
>
> On 14/05/20(Thu) 15:53, Vitaliy Makkoveev wrote:
>> Each `struct pppx_if' holds it's own `pipex_session' and this session is
>> used directly within ifnet's related handlers pppx_if_start() and
>&g
> On 23 May 2020, at 12:54, Martin Pieuchot wrote:
>
> On 22/05/20(Fri) 13:25, Vitaliy Makkoveev wrote:
>> On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote:
>>> [...]
>>> can you try the following diff?
>>>
>>
>> I tested th
> On 23 May 2020, at 13:11, Sergey Ryazanov wrote:
>
> Hello,
>
> On Wed, May 20, 2020 at 10:13 PM Vitaliy Makkoveev
> wrote:
>> On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote:
>>> On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev
>
On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote:
> On Wed, May 20, 2020 at 05:42:35PM +0300, Vitaliy Makkoveev wrote:
> > I got splassert with pppx(4) and net/ifq.c rev 1.38 raised by
> > NET_ASSERT_LOCKED() in netinet/ip_output.c:113 and underlaying routines.
> &g
After net/ifq.c rev 1.38 was reverted pppac(4) still has this problem.
pppac_output() called under NET_LOCK(). pppac_output() calls if_enqueue()
which calls ifq_start(). But now ifq_start() can run pppac_input()
directly under NET_LOCK() and also is can enqueue work and pppac_input()
will be
On Wed, May 20, 2020 at 04:08:01AM +0300, Sergey Ryazanov wrote:
> 2 Hi Vitaliy,
>
> On Tue, May 19, 2020 at 12:11 PM Vitaliy Makkoveev
> wrote:
> > On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote:
> > > Split checks from frame accepting with he
I got splassert with pppx(4) and net/ifq.c rev 1.38 raised by
NET_ASSERT_LOCKED() in netinet/ip_output.c:113 and underlaying routines.
net/ifq.c rev 1.38 is not in snapshot yet so you need to checkout and
build kernel to reproduce.
dmesg begin
splassert: ip_output: want 2 have 0
I am OK with this diff. Also all pseudo interfaces except switch(4) do
the same.
On Mon, May 04, 2020 at 10:02:53PM +0300, Sergey Ryazanov wrote:
> Use bpf filter hook from the common interface structure. This simplifies
> the code by unifying it and prepare ppp(4) for pipex(4) support.
>
> Ok?
Hello Sergey.
I am not the developer, but I works in pipex(4) layer too. Also mpi@
wants I did rewiev for your diffs.
On Mon, May 04, 2020 at 10:03:40PM +0300, Sergey Ryazanov wrote:
> Split checks from frame accepting with header removing in the common
> PPP input function. This should fix
Each `struct pppx_if' holds it's own `pipex_session' and this session is
used directly within ifnet's related handlers pppx_if_start() and
pppx_if_output().
pppx_if_destroy() at first destroys `pipex_session' and calls
if_deatch() which can cause context switch. Hypothetically,
pppx_if_start() or
Per thread cached credentials are accessed only by curproc. Curproc
doesn't modify it's 'p_ucred' directly. It allocates new copy, then
modyfies newcopy and replaces the old. So 'p_ucred' is owned by curproc.
Index: sys/sys/proc.h
Diff below is _not_ for commit. It's just an experiment I wish to share.
Socket layer is already protected by solock(). Inet sockets are
protected by NET_LOCK() which is grabbed by solock(). All other sockets
are still under KERNEL_LOCK().
My suggestion is to implement another lock, identical to
> On 12 Apr 2020, at 16:26, Martin Pieuchot wrote:
>
> The existing variations of the NET_LOCK() macros are confusing. We're
> now all aware of this fact. So let's keep them simple to prevent future
> mistakes :)
>
> The diff below reduces the current set of methods to the following:
>
>
it’s protects.
> On 11 Apr 2020, at 16:59, Vitaliy Makkoveev wrote:
>
> It protects nothing.
>
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.83
>
It protects nothing.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.83
diff -u -p -r1.83 if_pppx.c
--- sys/net/if_pppx.c 10 Apr 2020 07:36:52 - 1.83
+++ sys/net/if_pppx.c 11 Apr
Drop this diff please.
Subj.
Index: net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.83
diff -u -p -r1.83 if_pppx.c
--- net/if_pppx.c 10 Apr 2020 07:36:52 - 1.83
+++ net/if_pppx.c 10 Apr 2020 11:16:53
usb_detach_wait() will simply wait usb_detach_wakeup() for 60 sec. So
ugen_detach() will continue to destroy device context before threads
finish their io.
Index: sys/dev/usb/ugen.c
===
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
As mpi@ noticed malloc() with M_WAITOK can cause context switch too. I
wish to drop concurency while RBT pppx_ifs is accessed. So, remove
malloc() from pppx_if_find(). Looks like original code should do search
in the same way, but it didn't.
Index: sys/net/if_pppx.c
On Wed, Apr 08, 2020 at 11:35:06AM +0200, Martin Pieuchot wrote:
> On 08/04/20(Wed) 12:11, Vitaliy Makkoveev wrote:
> > On Wed, Apr 08, 2020 at 09:51:45AM +0200, Martin Pieuchot wrote:
> > [...]
> > As I see (pseudo code):
> > [...]
> > So, I fixed this issue
On Wed, Apr 08, 2020 at 09:51:45AM +0200, Martin Pieuchot wrote:
> On 07/04/20(Tue) 19:58, Vitaliy Makkoveev wrote:
> >
> >
> > > On 7 Apr 2020, at 17:43, Martin Pieuchot wrote:
> > >
> > > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
> > >
> On 7 Apr 2020, at 17:43, Martin Pieuchot wrote:
>
> On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
>> As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
>> code has some NET_LOCK() dances which make it unsafe. [...]
>
> The easiest
On Tue, Apr 07, 2020 at 06:38:11PM +0300, Vitaliy Makkoveev wrote:
> On Tue, Apr 07, 2020 at 04:43:55PM +0200, Martin Pieuchot wrote:
> > On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
> > > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
> >
On Tue, Apr 07, 2020 at 04:43:55PM +0200, Martin Pieuchot wrote:
> On 07/04/20(Tue) 17:14, Vitaliy Makkoveev wrote:
> > As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
> > code has some NET_LOCK() dances which make it unsafe. [...]
>
> T
Forgot to release lock in pppx_del_session() error case...
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.81
diff -u -p -r1.81 if_pppx.c
--- sys/net/if_pppx.c 7 Apr 2020 07:11:22 -
As Claudio Jeker noticed, NET_LOCK() can release KERNEL_LOCK(). pppx(4)
code has some NET_LOCK() dances which make it unsafe. Concurent thread
can receive CPU and enter to pppx_if_destroy() while we dance with
NET_LOCK(). The idea is to deny access to pxi at destruction stage.
If pxi_if is removed
On Tue, Apr 07, 2020 at 01:51:45PM +0300, Vitaliy Makkoveev wrote:
> On Tue, Apr 07, 2020 at 11:54:01AM +0200, Claudio Jeker wrote:
> > Unsure about this one here. I would prefer if the panic remained for now
> > (mainly because of the XXXSMP NET_UNLOCK() dance just a
On Tue, Apr 07, 2020 at 11:54:01AM +0200, Claudio Jeker wrote:
> Unsure about this one here. I would prefer if the panic remained for now
> (mainly because of the XXXSMP NET_UNLOCK() dance just above). I wonder if the
> order of this could not be modified so that the NET_LOCK is released after
>
pppx_if containing tree and per pppx_dev list are protected by rwlock so
these splx(9) related dances and commentaries are not actual.
Also pxd_svcq protected by NET_LOCK().
Index: sys/net/if_pppx.c
===
RCS file:
Just quick cleanup.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.79
diff -u -p -r1.79 if_pppx.c
--- sys/net/if_pppx.c 6 Apr 2020 12:31:30 - 1.79
+++ sys/net/if_pppx.c 6 Apr
> On 6 Apr 2020, at 17:37, Claudio Jeker wrote:
>
> On Mon, Apr 06, 2020 at 07:54:20PM +0300, Vitaliy Makkoveev wrote:
>> Deny to create pipex_session which is already exist. Newly created
>> session will be placed to list head so the caller of
>> pipex_*_lookup_s
Deny to create pipex_session which is already exist. Newly created
session will be placed to list head so the caller of
pipex_*_lookup_session() will receive wrong session.
Index: sys/net/if_pppx.c
===
RCS file:
Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.111
diff -u -p -r1.111 pipex.c
--- sys/net/pipex.c 6 Apr 2020 12:31:30 - 1.111
+++ sys/net/pipex.c 6 Apr 2020 12:58:31 -
@@
On Sat, Apr 04, 2020 at 06:52:49PM +0200, Martin Pieuchot wrote:
> On 02/04/20(Thu) 13:44, Vitaliy Makkoveev wrote:
> > This diff add ownership checks to the rest pipex_ioctl() commands. A few
> > words about pppx_get_closed(): since in-kernel timeout feature was
> > disable
On Fri, Apr 03, 2020 at 11:07:48AM +0200, Martin Pieuchot wrote:
> On 02/04/20(Thu) 13:44, Vitaliy Makkoveev wrote:
> > pipex(4) has pipex_ioctl() interface for pipex_session related routines.
> > pipex_ioctl() calls should be done with pipex_iface_contex, so any
> > oper
On Thu, Apr 02, 2020 at 05:06:30PM +0100, Iain R. Learmonth wrote:
> Hi,
>
> On 02/04/2020 12:47, Vitaliy Makkoveev wrote:
> > +.Xr pppax 4 ,
>
> I guess you meant pppac here.
>
> Thanks,
> Iain.
>
Thank
Sorry, screenshot was from pached kernel. Screenshot from clean kernel
included.
pipex(4) has pipex_ioctl() interface for pipex_session related routines.
pipex_ioctl() calls should be done with pipex_iface_contex, so any
operations should be done with pipex_sessions owned by passed
pipex_iface_contex. pipex_session check ownership is missing within
pipex_ioctl() so anybody can
On Thu, Apr 02, 2020 at 09:07:23AM +0200, Martin Pieuchot wrote:
> On 29/03/20(Sun) 00:16, Vitaliy Makkoveev wrote:
> > pipex not used with tun(4)
>
> It seems an oversight from the addition of the new pppac(4) driver. See
> net/if_tun.c commits from January this year.
>
On Thu, Apr 02, 2020 at 09:26:23AM +0200, Martin Pieuchot wrote:
> Hello Vitaliy,
>
> On 01/04/20(Wed) 12:59, Vitaliy Makkoveev wrote:
> > Updated diff. The idea is to use already existing pipex API for
> > pipex_session creation and destruction. pppx_if now holds a reference
Updated diff. The idea is to use already existing pipex API for
pipex_session creation and destruction. pppx_if now holds a reference
to pipex_session.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving
On Tue, Mar 31, 2020 at 06:15:46PM +0200, Martin Pieuchot wrote:
> On 31/03/20(Tue) 18:58, Vitaliy Makkoveev wrote:
> > On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> > > On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > > > I refactored pppx(
On Tue, Mar 31, 2020 at 05:23:46PM +0200, Martin Pieuchot wrote:
> On 31/03/20(Tue) 16:48, Vitaliy Makkoveev wrote:
> > I refactored pppx(4). The idea is to use pipex(4) as it was designed.
> > No one holds pipex_session outside pipex(4) so pipex_timer() calls are
> > safe. Un
I refactored pppx(4). The idea is to use pipex(4) as it was designed.
No one holds pipex_session outside pipex(4) so pipex_timer() calls are
safe. Unfortunately, this way gives some performance impact, because we
need to call pipex_lookup_by_session_id() in some places. This impact
will gone after
ping
pipex not used with tun(4)
Index: share/man/man4/pipex.4
===
RCS file: /cvs/src/share/man/man4/pipex.4,v
retrieving revision 1.11
diff -u -p -r1.11 pipex.4
--- share/man/man4/pipex.4 18 Apr 2017 03:21:48 - 1.11
+++
pppx(4) has code copypasted from pipex(4). Patch below deduplicates it.
Introduded pipex_session_setup() and pipex_session_destroy() functions.
Original pipex_destroy_session() renamed to pipex_del_session() to be
consistent with PIPEXDSESSION (Delete the specified session from the
kernel).
On Fri, Mar 27, 2020 at 03:13:01PM +0100, Martin Pieuchot wrote:
> On 27/03/20(Fri) 15:16, Vitaliy Makkoveev wrote:
> > On Fri, Mar 27, 2020 at 10:43:52AM +0100, Martin Pieuchot wrote:
> > > Do you have a backtrace for the memory corruption? Could you share it?
> > Yes. A
On Fri, Mar 27, 2020 at 10:43:52AM +0100, Martin Pieuchot wrote:
> Do you have a backtrace for the memory corruption? Could you share it?
Yes. Apply path below, compile and run code, and when you had see
"pipex_session ... killed" kill this code. Screenshot attached.
STABLE-6.[56] are affected
On Fri, Mar 27, 2020 at 03:16:54PM +0300, Vitaliy Makkoveev wrote:
> On Fri, Mar 27, 2020 at 10:43:52AM +0100, Martin Pieuchot wrote:
> > Do you have a backtrace for the memory corruption? Could you share it?
> Yes. Apply path below, compile and run code, and when you had see
>
Each pipex_session has timeout_sec field and if it is not 0,
pipex_timer() can destroy pipex_session. Each pppx_if contents
pipex_session. If userland creates pppx_if and pipex_session has
timeout (for example, npppd.conf has idle-timeout option), pipex_timer()
can destroy this pipex_session and
On Thu, Mar 26, 2020 at 01:46:29PM +0100, Martin Pieuchot wrote:
> Does the diff below works for you? Are you ok with the direction? Any
> comment?
Diff works for me, Except you missed switch in the and of
pppx_add_session() and pipex_add_session().
Index: sys/net/if_pppx.c
On Thu, Mar 26, 2020 at 11:56:27AM +0100, Martin Pieuchot wrote:
> On 26/03/20(Thu) 13:34, Vitaliy Makkoveev wrote:
> > Add missing #ifdefs to pppx_if_destroy() as it done in
> > pipex_destroy_session(). Also remove unnecessary cast.
>
> What's the point of such #ifdef? I u
Add missing #ifdefs to pppx_if_destroy() as it done in
pipex_destroy_session(). Also remove unnecessary cast.
Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.76
diff -u -p -r1.76 if_pppx.c
pipex_destroy_session should be called under NET_LOCK but if it called
by this sequence: pppacclose -> pipex_iface_fini -> pipex_iface_stop
-> pipex_destroy_session, NET_LOCK is missing and kernel crashes.
pipex_iface_stop calls are protected by NET_LOCK, so it should be also
protected within
> On 19 Feb 2020, at 20:51, Martin Pieuchot wrote:
>
> On 19/02/20(Wed) 14:13, Vitaliy Makkoveev wrote:
>> On Wed, Jan 17, 2018 at 11:40:24AM +0100, Martin Pieuchot wrote:
>>> Hello Sebastien,
>>>
>>> On 17/01/18(Wed) 10:19, Sebastien Marie w
On Wed, Jan 17, 2018 at 11:40:24AM +0100, Martin Pieuchot wrote:
> Hello Sebastien,
>
> On 17/01/18(Wed) 10:19, Sebastien Marie wrote:
> > [...]
> > kernel modification is desirable in some cases, at least for disabling
> > ulpt(4) when using cups with USB printer.
>
> Sorry to hijack your
Unfortunately, in 2 cases this diff will increase needed variable
for non existing files too. This is the fixed version.
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.284
diff -u
I have questions about sysctl_file():
1. Looks like sysctl_file() should do the same things for KERN_FILE_BYPID and
KERN_FILE_BYUID cases. But pr-ps_textvp (vnode of executable) will be copied
for KERN_FILE_BYPID case only. Is this copying missing for KERN_FILE_BYUID?
2. (arg -1) check looks be
In the not far future fd_getfile() will return the referenced file
instance, so dupfdopen() should be modified for the same reasons
that getsock() and getvnode().
The modified dupfdopen() receives a thread pointer instead of it's
file descriptor table and of it's file descriptor for duplication.
Struct file has reference count f_count field. When f_count 0 file instance
exists, when f_count becomes 0 file instance should be removed. It's simple
but some strange magic exists. When file was created by falloc() it has
f_count == 2. One additional FREF() call was done inside falloc(). After
On 30 Apr 2015, at 12:49, Martin Pieuchot m...@openbsd.org wrote:
Your diff makes sense but sadly it is broken, could you send a diff that
can be applied?
Index: share/man/man9/file.9
===
RCS file:
Mail.app removes spaces from line start :(
Now fd_getfile() function returns unacquired struct file instance and the
typical usage is:
struct file *fp;
if ((fp = fd_getfile(fpd, fd)) == NULL)
return ENOENT;
if (/* obtained fp is unacceptable */)
return EERROR;
FREF(fp);
701 - 774 of 774 matches
Mail list logo
