doas called multiple times hangs

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

doas called multiple times hangs

dsendkowski
Hi,

Calling 'doas' in a loop makes the machine hang.
I guess this is not an expected behavior.
It can be checked by executing the following simple bash script:

for i in {0..20000}
do
doas ls some_dir
done

I have run it on three different machines and the result is the same on
each of them. After about 9000 iterations the entire machine hangs.
Executing the script without 'doas' works fine.
Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

Hiltjo Posthuma
On Sun, Jan 20, 2019 at 11:15:38AM +0100, Dariusz Sendkowski wrote:

> Hi,
>
> Calling 'doas' in a loop makes the machine hang.
> I guess this is not an expected behavior.
> It can be checked by executing the following simple bash script:
>
> for i in {0..20000}
> do
> doas ls some_dir
> done
>
> I have run it on three different machines and the result is the same on
> each of them. After about 9000 iterations the entire machine hangs.
> Executing the script without 'doas' works fine.

Hi,

Just tested, but no issue here on -current.

What is your doas.conf contents and OpenBSD version / dmesg?

--
Kind regards,
Hiltjo

Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

dsendkowski
This is -stable 6.4.
doas.conf:
permit nopass myuser as root




On Sun, 20 Jan 2019 at 11:50, Hiltjo Posthuma <[hidden email]>
wrote:

> On Sun, Jan 20, 2019 at 11:15:38AM +0100, Dariusz Sendkowski wrote:
> > Hi,
> >
> > Calling 'doas' in a loop makes the machine hang.
> > I guess this is not an expected behavior.
> > It can be checked by executing the following simple bash script:
> >
> > for i in {0..20000}
> > do
> > doas ls some_dir
> > done
> >
> > I have run it on three different machines and the result is the same on
> > each of them. After about 9000 iterations the entire machine hangs.
> > Executing the script without 'doas' works fine.
>
> Hi,
>
> Just tested, but no issue here on -current.
>
> What is your doas.conf contents and OpenBSD version / dmesg?
>
> --
> Kind regards,
> Hiltjo
>
Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

dsendkowski
I investigated the problem a little more and found, that when there is a
non-existent directory entry in my PATH, the problem occurs. If all of the
directories from my PATH exist, then it works fine.



niedz., 20 sty 2019 o 12:00 Dariusz Sendkowski <[hidden email]>
napisał(a):

> This is -stable 6.4.
> doas.conf:
> permit nopass myuser as root
>
>
>
>
> On Sun, 20 Jan 2019 at 11:50, Hiltjo Posthuma <[hidden email]>
> wrote:
>
>> On Sun, Jan 20, 2019 at 11:15:38AM +0100, Dariusz Sendkowski wrote:
>> > Hi,
>> >
>> > Calling 'doas' in a loop makes the machine hang.
>> > I guess this is not an expected behavior.
>> > It can be checked by executing the following simple bash script:
>> >
>> > for i in {0..20000}
>> > do
>> > doas ls some_dir
>> > done
>> >
>> > I have run it on three different machines and the result is the same on
>> > each of them. After about 9000 iterations the entire machine hangs.
>> > Executing the script without 'doas' works fine.
>>
>> Hi,
>>
>> Just tested, but no issue here on -current.
>>
>> What is your doas.conf contents and OpenBSD version / dmesg?
>>
>> --
>> Kind regards,
>> Hiltjo
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

Ted Unangst-6
Dariusz Sendkowski wrote:
> I investigated the problem a little more and found, that when there is a
> non-existent directory entry in my PATH, the problem occurs. If all of the
> directories from my PATH exist, then it works fine.

To help isolate the problem, if you disable unveil, does it work?


Index: doas.c
===================================================================
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.74
diff -u -p -r1.74 doas.c
--- doas.c 17 Jan 2019 05:35:35 -0000 1.74
+++ doas.c 20 Jan 2019 21:30:55 -0000
@@ -409,8 +409,6 @@ main(int argc, char **argv)
  if (setenv("PATH", safepath, 1) == -1)
  err(1, "failed to set PATH '%s'", safepath);
  }
- if (unveilcommands(getenv("PATH"), cmd) == 0)
- goto fail;
 
  if (pledge("stdio rpath getpw exec id", NULL) == -1)
  err(1, "pledge");

Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

dsendkowski
Yes, it does.

I extracted 'unveilcommands' function from doas.c and put it into a
standalone program to run it.
It turned out the result was the same as in doas command. When I disable
unveil, then it works fine.





niedz., 20 sty 2019 o 22:31 Ted Unangst <[hidden email]> napisał(a):

> Dariusz Sendkowski wrote:
> > I investigated the problem a little more and found, that when there is a
> > non-existent directory entry in my PATH, the problem occurs. If all of
> the
> > directories from my PATH exist, then it works fine.
>
> To help isolate the problem, if you disable unveil, does it work?
>
>
> Index: doas.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 doas.c
> --- doas.c      17 Jan 2019 05:35:35 -0000      1.74
> +++ doas.c      20 Jan 2019 21:30:55 -0000
> @@ -409,8 +409,6 @@ main(int argc, char **argv)
>                 if (setenv("PATH", safepath, 1) == -1)
>                         err(1, "failed to set PATH '%s'", safepath);
>         }
> -       if (unveilcommands(getenv("PATH"), cmd) == 0)
> -               goto fail;
>
>         if (pledge("stdio rpath getpw exec id", NULL) == -1)
>                 err(1, "pledge");
>
Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

Ted Unangst-6
Dariusz Sendkowski wrote:
> Yes, it does.
>
> I extracted 'unveilcommands' function from doas.c and put it into a
> standalone program to run it.
> It turned out the result was the same as in doas command. When I disable
> unveil, then it works fine.

This diff should fix the problem.


Index: kern_unveil.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.22
diff -u -p -r1.22 kern_unveil.c
--- kern_unveil.c 17 Jan 2019 03:26:19 -0000 1.22
+++ kern_unveil.c 21 Jan 2019 01:31:23 -0000
@@ -630,8 +630,6 @@ unveil_add(struct proc *p, struct nameid
  done:
  if (ret == 0)
  unveil_add_traversed_vnodes(p, ndp);
- unveil_free_traversed_vnodes(ndp);
- pool_put(&namei_pool, ndp->ni_cnd.cn_pnbuf);
  return ret;
 }
 
Index: vfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.310
diff -u -p -r1.310 vfs_syscalls.c
--- vfs_syscalls.c 3 Jan 2019 21:52:31 -0000 1.310
+++ vfs_syscalls.c 21 Jan 2019 01:29:38 -0000
@@ -92,6 +92,7 @@ int dofutimens(struct proc *, int, struc
 int dounmount_leaf(struct mount *, int, struct proc *);
 int unveil_add(struct proc *, struct nameidata *, const char *);
 void unveil_removevnode(struct vnode *vp);
+void unveil_free_traversed_vnodes(struct nameidata *);
 ssize_t unveil_find_cover(struct vnode *, struct proc *);
 struct unveil *unveil_lookup(struct vnode *, struct proc *, ssize_t *);
 
@@ -948,6 +949,8 @@ sys_unveil(struct proc *p, void *v, regi
  vrele(nd.ni_vp);
  if (nd.ni_dvp && nd.ni_dvp != nd.ni_vp)
  vrele(nd.ni_dvp);
+ unveil_free_traversed_vnodes(&nd);
+ pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
 
  return (error);
 }

Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

Ted Unangst-6
Ted Unangst wrote:
> Dariusz Sendkowski wrote:
> > Yes, it does.
> >
> > I extracted 'unveilcommands' function from doas.c and put it into a
> > standalone program to run it.
> > It turned out the result was the same as in doas command. When I disable
> > unveil, then it works fine.
>
> This diff should fix the problem.

Actually, miscalculation. This is a better diff. Sorry for the trouble.
Against current, but should be adaptable to stable.

Index: vfs_syscalls.c
===================================================================
RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.310
diff -u -p -r1.310 vfs_syscalls.c
--- vfs_syscalls.c 3 Jan 2019 21:52:31 -0000 1.310
+++ vfs_syscalls.c 21 Jan 2019 04:57:17 -0000
@@ -92,6 +92,7 @@ int dofutimens(struct proc *, int, struc
 int dounmount_leaf(struct mount *, int, struct proc *);
 int unveil_add(struct proc *, struct nameidata *, const char *);
 void unveil_removevnode(struct vnode *vp);
+void unveil_free_traversed_vnodes(struct nameidata *);
 ssize_t unveil_find_cover(struct vnode *, struct proc *);
 struct unveil *unveil_lookup(struct vnode *, struct proc *, ssize_t *);
 
@@ -911,7 +912,7 @@ sys_unveil(struct proc *p, void *v, regi
 
  nd.ni_pledge = PLEDGE_UNVEIL;
  if ((error = namei(&nd)) != 0)
- return (error);
+ goto end;
 
  /*
  * XXX Any access to the file or directory will allow us to
@@ -948,6 +949,10 @@ sys_unveil(struct proc *p, void *v, regi
  vrele(nd.ni_vp);
  if (nd.ni_dvp && nd.ni_dvp != nd.ni_vp)
  vrele(nd.ni_dvp);
+
+ pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
+end:
+ unveil_free_traversed_vnodes(&nd);
 
  return (error);
 }
Index: kern_unveil.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_unveil.c,v
retrieving revision 1.22
diff -u -p -r1.22 kern_unveil.c
--- kern_unveil.c 17 Jan 2019 03:26:19 -0000 1.22
+++ kern_unveil.c 21 Jan 2019 05:01:26 -0000
@@ -630,8 +630,6 @@ unveil_add(struct proc *p, struct nameid
  done:
  if (ret == 0)
  unveil_add_traversed_vnodes(p, ndp);
- unveil_free_traversed_vnodes(ndp);
- pool_put(&namei_pool, ndp->ni_cnd.cn_pnbuf);
  return ret;
 }
 

Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

dsendkowski
I applied this patch, as is, to the stable sources and it works now.
Thanks.



pon., 21 sty 2019 o 06:03 Ted Unangst <[hidden email]> napisał(a):

> Ted Unangst wrote:
> > Dariusz Sendkowski wrote:
> > > Yes, it does.
> > >
> > > I extracted 'unveilcommands' function from doas.c and put it into a
> > > standalone program to run it.
> > > It turned out the result was the same as in doas command. When I
> disable
> > > unveil, then it works fine.
> >
> > This diff should fix the problem.
>
> Actually, miscalculation. This is a better diff. Sorry for the trouble.
> Against current, but should be adaptable to stable.
>
> Index: vfs_syscalls.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> retrieving revision 1.310
> diff -u -p -r1.310 vfs_syscalls.c
> --- vfs_syscalls.c      3 Jan 2019 21:52:31 -0000       1.310
> +++ vfs_syscalls.c      21 Jan 2019 04:57:17 -0000
> @@ -92,6 +92,7 @@ int dofutimens(struct proc *, int, struc
>  int dounmount_leaf(struct mount *, int, struct proc *);
>  int unveil_add(struct proc *, struct nameidata *, const char *);
>  void unveil_removevnode(struct vnode *vp);
> +void unveil_free_traversed_vnodes(struct nameidata *);
>  ssize_t unveil_find_cover(struct vnode *, struct proc *);
>  struct unveil *unveil_lookup(struct vnode *, struct proc *, ssize_t *);
>
> @@ -911,7 +912,7 @@ sys_unveil(struct proc *p, void *v, regi
>
>         nd.ni_pledge = PLEDGE_UNVEIL;
>         if ((error = namei(&nd)) != 0)
> -               return (error);
> +               goto end;
>
>         /*
>          * XXX Any access to the file or directory will allow us to
> @@ -948,6 +949,10 @@ sys_unveil(struct proc *p, void *v, regi
>                 vrele(nd.ni_vp);
>         if (nd.ni_dvp && nd.ni_dvp != nd.ni_vp)
>                 vrele(nd.ni_dvp);
> +
> +       pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
> +end:
> +       unveil_free_traversed_vnodes(&nd);
>
>         return (error);
>  }
> Index: kern_unveil.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_unveil.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 kern_unveil.c
> --- kern_unveil.c       17 Jan 2019 03:26:19 -0000      1.22
> +++ kern_unveil.c       21 Jan 2019 05:01:26 -0000
> @@ -630,8 +630,6 @@ unveil_add(struct proc *p, struct nameid
>   done:
>         if (ret == 0)
>                 unveil_add_traversed_vnodes(p, ndp);
> -       unveil_free_traversed_vnodes(ndp);
> -       pool_put(&namei_pool, ndp->ni_cnd.cn_pnbuf);
>         return ret;
>  }
>
>
Reply | Threaded
Open this post in threaded view
|

Re: doas called multiple times hangs

Hiltjo Posthuma
On Mon, Jan 21, 2019 at 11:06:58AM +0100, Dariusz Sendkowski wrote:
> I applied this patch, as is, to the stable sources and it works now.
> Thanks.
>
>

I've tested this patch too on 6.4 on amd64 and it seems fixed now.

Thanks Ted for the patch :)


A quick little program to reproduce the issue:

#include <stdio.h>
#include <unistd.h>

int
main(void)
{
        int i;

        for (i = 0; i < 20000; ++i) {
                printf("%d\n", i);
                unveil("/nonexistant/ls", "x");
        }

        return 0;
}

>
> pon., 21 sty 2019 o 06:03 Ted Unangst <[hidden email]> napisał(a):
>
> > Ted Unangst wrote:
> > > Dariusz Sendkowski wrote:
> > > > Yes, it does.
> > > >
> > > > I extracted 'unveilcommands' function from doas.c and put it into a
> > > > standalone program to run it.
> > > > It turned out the result was the same as in doas command. When I
> > disable
> > > > unveil, then it works fine.
> > >
> > > This diff should fix the problem.
> >
> > Actually, miscalculation. This is a better diff. Sorry for the trouble.
> > Against current, but should be adaptable to stable.
> >
> > Index: vfs_syscalls.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/vfs_syscalls.c,v
> > retrieving revision 1.310
> > diff -u -p -r1.310 vfs_syscalls.c
> > --- vfs_syscalls.c      3 Jan 2019 21:52:31 -0000       1.310
> > +++ vfs_syscalls.c      21 Jan 2019 04:57:17 -0000
> > @@ -92,6 +92,7 @@ int dofutimens(struct proc *, int, struc
> >  int dounmount_leaf(struct mount *, int, struct proc *);
> >  int unveil_add(struct proc *, struct nameidata *, const char *);
> >  void unveil_removevnode(struct vnode *vp);
> > +void unveil_free_traversed_vnodes(struct nameidata *);
> >  ssize_t unveil_find_cover(struct vnode *, struct proc *);
> >  struct unveil *unveil_lookup(struct vnode *, struct proc *, ssize_t *);
> >
> > @@ -911,7 +912,7 @@ sys_unveil(struct proc *p, void *v, regi
> >
> >         nd.ni_pledge = PLEDGE_UNVEIL;
> >         if ((error = namei(&nd)) != 0)
> > -               return (error);
> > +               goto end;
> >
> >         /*
> >          * XXX Any access to the file or directory will allow us to
> > @@ -948,6 +949,10 @@ sys_unveil(struct proc *p, void *v, regi
> >                 vrele(nd.ni_vp);
> >         if (nd.ni_dvp && nd.ni_dvp != nd.ni_vp)
> >                 vrele(nd.ni_dvp);
> > +
> > +       pool_put(&namei_pool, nd.ni_cnd.cn_pnbuf);
> > +end:
> > +       unveil_free_traversed_vnodes(&nd);
> >
> >         return (error);
> >  }
> > Index: kern_unveil.c
> > ===================================================================
> > RCS file: /cvs/src/sys/kern/kern_unveil.c,v
> > retrieving revision 1.22
> > diff -u -p -r1.22 kern_unveil.c
> > --- kern_unveil.c       17 Jan 2019 03:26:19 -0000      1.22
> > +++ kern_unveil.c       21 Jan 2019 05:01:26 -0000
> > @@ -630,8 +630,6 @@ unveil_add(struct proc *p, struct nameid
> >   done:
> >         if (ret == 0)
> >                 unveil_add_traversed_vnodes(p, ndp);
> > -       unveil_free_traversed_vnodes(ndp);
> > -       pool_put(&namei_pool, ndp->ni_cnd.cn_pnbuf);
> >         return ret;
> >  }
> >
> >

--
Kind regards,
Hiltjo