Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-26 Thread Dmitry Chestnykh
On Mon, Jun 26, 2017 at 5:45 PM, Todd C. Miller wrote: > On Mon, 26 Jun 2017 08:50:30 -0600, "Todd C. Miller" wrote: > >> On Sun, 25 Jun 2017 14:34:40 -0400, "Ted Unangst" wrote: >> >> > will this cause problems if a number repeats? we've seen problems with that >> >

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-26 Thread Todd C. Miller
On Mon, 26 Jun 2017 08:50:30 -0600, "Todd C. Miller" wrote: > On Sun, 25 Jun 2017 14:34:40 -0400, "Ted Unangst" wrote: > > > will this cause problems if a number repeats? we've seen problems with that > > before, where you get a sequence like 4, 7, 4 and then bad things happen. > > Yes, that is

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-26 Thread Todd C. Miller
On Sun, 25 Jun 2017 14:34:40 -0400, "Ted Unangst" wrote: > will this cause problems if a number repeats? we've seen problems with that > before, where you get a sequence like 4, 7, 4 and then bad things happen. Yes, that is why it currently just increments. A linear congruential generator like

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-26 Thread Klemens Nanni
On Sun, Jun 25, 2017 at 11:21:50PM -0600, Theo de Raadt wrote: On Sun, Jun 25, 2017 at 10:47:08PM -0600, Theo de Raadt wrote: >> :-) Speaking of signed integers, does it really need to be signed? > >Perhaps not. Anyone know for sure? > >Of course this number should probably exclude 0 in it's

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Theo de Raadt
> On Sun, Jun 25, 2017 at 10:47:08PM -0600, Theo de Raadt wrote: > >> :-) Speaking of signed integers, does it really need to be signed? > > > >Perhaps not. Anyone know for sure? > > > >Of course this number should probably exclude 0 in it's range. > Probably [0, 3] completely as those are

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Klemens Nanni
On Sun, Jun 25, 2017 at 10:47:08PM -0600, Theo de Raadt wrote: :-) Speaking of signed integers, does it really need to be signed? Perhaps not. Anyone know for sure? Of course this number should probably exclude 0 in it's range. Probably [0, 3] completely as those are reserved for

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Theo de Raadt
> :-) Speaking of signed integers, does it really need to be signed? Perhaps not. Anyone know for sure? Of course this number should probably exclude 0 in it's range.

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Dmitry Chestnykh
Here's another version: it starts with a random 31-bit integer and then for subsequent allocations sets 23 random bits and incremented 8 bits from the previous generation number, so it's guaranteed to not repeat for at least 256 allocations (suggested by Kai Antweiler). -Dmitry Index:

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Dmitry Chestnykh
On Sun, Jun 25, 2017 at 8:34 PM, Ted Unangst wrote: >> This patch always assigns a random non-zero positive integer in [1, >> INT_MAX] range, not equal to the previous generation number. > > will this cause problems if a number repeats? we've seen problems with that > before,

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Dmitry Chestnykh
On Sun, Jun 25, 2017 at 8:20 PM, Theo de Raadt wrote: > I'd really prefer to see a construct which doesn't do that. But > I don't know if that would result in something really complex. Sure, will send a different version if the problem that tedu@ mentioned can be solved.

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Ted Unangst
Dmitry Chestnykh wrote: > Hello, > > In ffs_inode_alloc(), inode generation numbers are incremented after > being assigned an initial random value. Since the di_gen field is a > signed integer, it may overflow, causing undefined behavior (this is > unlikely to ever happen, though). > > This

Re: [PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Theo de Raadt
+ do { + ngen = arc4random_uniform(INT_MAX) + 1; + } while (DIP(ip, gen) == ngen); arc4random_uniform contains a potential loop (approx 1 in 4billion) for uniform resolution, and this wraps it with another loop (approx 1 in 4billion). I'd really prefer to see a

[PATCH] ffs: always assign random inode generation numbers

2017-06-25 Thread Dmitry Chestnykh
Hello, In ffs_inode_alloc(), inode generation numbers are incremented after being assigned an initial random value. Since the di_gen field is a signed integer, it may overflow, causing undefined behavior (this is unlikely to ever happen, though). This patch always assigns a random non-zero