Re: opendev(3) tweak

2016-06-10 Thread Sebastien Marie
On Thu, Jun 09, 2016 at 09:19:30PM +0200, Theo Buehler wrote: > On Tue, Mar 15, 2016 at 12:32:16PM -0600, Theo de Raadt wrote: > > I am simply saying that pledge before opendev() makes no sense, > > because opendev() does not gaurantee the type of descriptor it is > > opening. > > I noticed that

Re: opendev(3) tweak

2016-06-09 Thread Theo Buehler
On Tue, Mar 15, 2016 at 12:32:16PM -0600, Theo de Raadt wrote: > I am simply saying that pledge before opendev() makes no sense, > because opendev() does not gaurantee the type of descriptor it is > opening. I noticed that this patch is still uncommitted since nobody ok'd it. Sorry about that.

Re: opendev(3) tweak

2016-03-15 Thread Theo de Raadt
> On Mon, Mar 14, 2016 at 10:19:53PM +0100, Theo Buehler wrote: > > On Thu, Mar 10, 2016 at 12:52:35PM +0100, Marc Espie wrote: > > > Already shown to a few people, but since pledge(2) aborts on non-dev, > > > let's > > > check upfront that we're of the right type. > > > > > > I don't think this

Re: opendev(3) tweak

2016-03-15 Thread Alexander Hall
I'm aware I'm kicking an old horse here, but... On Thu, Mar 10, 2016 at 12:52:35PM +0100, Marc Espie wrote: > @@ -106,5 +108,17 @@ opendev(const char *path, int oflags, in > if (realpath) > *realpath = namebuf; If anything like this goes in (or did it already?) the *realpath

Re: opendev(3) tweak

2016-03-14 Thread Marc Espie
On Mon, Mar 14, 2016 at 10:19:53PM +0100, Theo Buehler wrote: > On Thu, Mar 10, 2016 at 12:52:35PM +0100, Marc Espie wrote: > > Already shown to a few people, but since pledge(2) aborts on non-dev, let's > > check upfront that we're of the right type. > > > > I don't think this requires a bump.

Re: opendev(3) tweak

2016-03-14 Thread Theo Buehler
On Thu, Mar 10, 2016 at 12:52:35PM +0100, Marc Espie wrote: > Already shown to a few people, but since pledge(2) aborts on non-dev, let's > check upfront that we're of the right type. > > I don't think this requires a bump. It doesn't really change the interface, > just makes it stricter. > If

Re: opendev(3) tweak

2016-03-10 Thread Theo de Raadt
I am compelled to add two throughts about opendev() and pledge: Beforehands, please read src/lib/libutil/opendev.c I am not saying opendev is wrong, the design of opening a master device, doing an ioctl, and then finding the correct device to actually open was very expedient, DUID development

Re: opendev(3) tweak

2016-03-10 Thread Theo de Raadt
> On Thu, Mar 10, 2016 at 08:48:21AM -0700, Theo de Raadt wrote: > > The reason for these checks is because they protect the kernel, > > and they identify a program that does the wrong thing. Here, a > > program did the wrong thing. I am 100% in agreement that opendev > > may not be the right

Re: opendev(3) tweak

2016-03-10 Thread Marc Espie
On Thu, Mar 10, 2016 at 08:48:21AM -0700, Theo de Raadt wrote: > The reason for these checks is because they protect the kernel, > and they identify a program that does the wrong thing. Here, a > program did the wrong thing. I am 100% in agreement that opendev > may not be the right place to do

Re: opendev(3) tweak

2016-03-10 Thread Theo de Raadt
> So I think we need to narrow down the pledge(2) semantics a bit more > with respect to ioctls. I'm inclined to say that if a certain ioctl > is allowed by pledge(2) it should not abort the program anymore but > return an error like it would do if unpledged. But perhaps we need to > make that

Re: opendev(3) tweak

2016-03-10 Thread Theo de Raadt
> Checks like the one you introduce here suffer from TOCTOU. I don't see that. It is not a stat, it is an fstat. The descriptor opened early, remains the same type through the whole operation.

Re: opendev(3) tweak

2016-03-10 Thread Mark Kettenis
> Date: Thu, 10 Mar 2016 12:52:35 +0100 > From: Marc Espie > > Already shown to a few people, but since pledge(2) aborts on non-dev, let's > check upfront that we're of the right type. > > I don't think this requires a bump. It doesn't really change the interface, > just makes

opendev(3) tweak

2016-03-10 Thread Marc Espie
Already shown to a few people, but since pledge(2) aborts on non-dev, let's check upfront that we're of the right type. I don't think this requires a bump. It doesn't really change the interface, just makes it stricter. Index: opendev.3