Source-Changes-D archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src/sys/kern





On 28/08/2018 08:33, Maxime Villard wrote:
Le 28/08/2018 à 08:38, Nick Hudson a écrit :
On 25/08/2018 14:50, Maxime Villard wrote:
Le 25/08/2018 à 14:37, Nick Hudson a écrit :
On 25 Aug 2018, at 10:58, Maxime Villard <max%m00nbsd.net@localhost> wrote:
Le 25/08/2018 à 11:50, Nick Hudson a écrit :
Reversed align and align_offset arguments to pool_cache_bootstrap appear to be
the problem.

indeed

I rushed this email out... I think pool red zone needs fixing for non zero
align_offset?

I don't think so, unless you see a specific problem.

pool_redzone_init() only modifies pr_size. align_offset is touched afterwards, depending on pr_size, and I don't see why we would need to take care of it.

I hardly see how the inverted parameters can be correct. Already in the code we compute (align - ioff), so if you pass align < ioff, we have a problem.

Let me try this again...

There is clearly a problem with POOL_REDZONE and align < align_offset. Currently subr_pool.c has

585:         * Silently enforce `0 <= ioff < align'.
586:         */
587:        pp->pr_itemoffset = ioff %= align;

Either this is made loud or POOL_REDZONE is fixed - which do you prefer?

I would prefer that you explain what is wrong with the line of code you
quoted, because I've read it, and I still don't see what is "clearly"
problematic.

Remember where we started with this... An RPI would not boot correctly.  I tracked this down to pool_redzone_fill overwriting the l2 page table of the init process. To me this clearly demonstrates
that POOL_REDZONE code is problematic with align < align_offset

I'm not saying that "silently enforcing '0 <= ioff < align' is a problem on its own, but a problem when
combined with POOL_REDZONE.

Nick



Home | Main Index | Thread Index | Old Index