On Tue, Jun 18, 2024 at 10:35 AM Michal Hocko <mhocko@xxxxxxxx> wrote:
>
> On Mon 17-06-24 11:04:41, Tim Chen wrote:
> > diff --git a/kernel/sys.c b/kernel/sys.c
> > index 3a2df1bd9f64..b4e523728c3e 100644
> > --- a/kernel/sys.c
> > +++ b/kernel/sys.c
> > @@ -1471,6 +1471,7 @@ static int do_prlimit(struct task_struct *tsk, unsigned int resource,
> > return -EINVAL;
> > resource = array_index_nospec(resource, RLIM_NLIMITS);
> >
> > + task_lock(tsk->group_leader);
> > if (new_rlim) {
> > if (new_rlim->rlim_cur > new_rlim->rlim_max)
> > return -EINVAL;
>
> This is clearly broken as it leaves the lock behind on the error, no?
As I explained in my other e-mail there is no need to synchronize
against rlimit changes, merely the code needs to honor the observed
value, whatever it is.
This holds for the stock kernel, does not hold for v1 of the patchset
and presumably will be addressed in v2.
Even if some synchronization was required, moving the lock higher does
not buy anything because the newly protected area only validates the
new limit before there is any attempt at setting it and the old limit
is not looked at.
I think we can safely drop this area and patiently wait for v2.
--
Mateusz Guzik <mjguzik gmail.com>
