Hi Greg,
> In the Linux kernel, the following vulnerability has been resolved:
>
> tracing/trigger: Fix to return error if failed to alloc snapshot
>
> Fix register_snapshot_trigger() to return error code if it failed to
> allocate a snapshot instead of 0 (success). Unless that, it will register
> snapshot trigger without an error.
This commit is problematic on 4.19.y, 5.4.y, 5.10.y, and 5.15.y,
and should be reversed, and this CVE should be rejected for those
versions.
The return value should be 0 on failure, because in the functions
event_trigger_callback() and event_enable_trigger_func(), we have:
ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
/*
* The above returns on success the # of functions enabled,
* but if it didn't find any functions it returns zero.
* Consider no functions a failure too.
*/
if (!ret) {
ret = -ENOENT;
Thus, the commit breaks this assumption.
This commit needs b8cc44a4d3c1 ("tracing: Remove logic for registering
multiple event triggers at a time") as a prerequisite, as it removes
the above.
Thanks,
Siddh
On Thu, Apr 18, 2024 at 11:59:41AM +0000, Siddh Raman Pant wrote:
> Hi Greg,
>
> > In the Linux kernel, the following vulnerability has been resolved:
> >
> > tracing/trigger: Fix to return error if failed to alloc snapshot
> >
> > Fix register_snapshot_trigger() to return error code if it failed to
> > allocate a snapshot instead of 0 (success). Unless that, it will register
> > snapshot trigger without an error.
>
> This commit is problematic on 4.19.y, 5.4.y, 5.10.y, and 5.15.y,
> and should be reversed, and this CVE should be rejected for those
> versions.
Then please submit a patch for this.
But note, CVEs are not for specific versions, sorry. We give a hint as
to what kernel versions might be affected, but we don not assign CVE to
versions.
>
> The return value should be 0 on failure, because in the functions
> event_trigger_callback() and event_enable_trigger_func(), we have:
>
> ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> /*
> * The above returns on success the # of functions enabled,
> * but if it didn't find any functions it returns zero.
> * Consider no functions a failure too.
> */
> if (!ret) {
> ret = -ENOENT;
>
> Thus, the commit breaks this assumption.
>
> This commit needs b8cc44a4d3c1 ("tracing: Remove logic for registering
> multiple event triggers at a time") as a prerequisite, as it removes
> the above.
Should we just take that patch instead?
thanks,
greg k-h
On Thu, Apr 18 2024 at 14:34:57 +0200, [email protected]
wrote:
> On Thu, Apr 18, 2024 at 11:59:41AM +0000, Siddh Raman Pant wrote:
> > Hi Greg,
> >
> > > In the Linux kernel, the following vulnerability has been resolved:
> > >
> > > tracing/trigger: Fix to return error if failed to alloc snapshot
> > >
> > > Fix register_snapshot_trigger() to return error code if it failed to
> > > allocate a snapshot instead of 0 (success). Unless that, it will register
> > > snapshot trigger without an error.
> >
> > This commit is problematic on 4.19.y, 5.4.y, 5.10.y, and 5.15.y,
> > and should be reversed, and this CVE should be rejected for those
> > versions.
>
> Then please submit a patch for this.
Sure.
> But note, CVEs are not for specific versions, sorry. We give a hint as
> to what kernel versions might be affected, but we don not assign CVE to
> versions.
Cool.
> >
> > The return value should be 0 on failure, because in the functions
> > event_trigger_callback() and event_enable_trigger_func(), we have:
> >
> > ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> > /*
> > * The above returns on success the # of functions enabled,
> > * but if it didn't find any functions it returns zero.
> > * Consider no functions a failure too.
> > */
> > if (!ret) {
> > ret = -ENOENT;
> >
> > Thus, the commit breaks this assumption.
> >
> > This commit needs b8cc44a4d3c1 ("tracing: Remove logic for registering
> > multiple event triggers at a time") as a prerequisite, as it removes
> > the above.
>
> Should we just take that patch instead?
The series in which the patch is posted is here:
- https://lore.kernel.org/lkml/[email protected]/
- https://lore.kernel.org/lkml/[email protected]/
Seems like some good tracing subsystem refactoring. So if I understand
Documentation/process/stable-kernel-rules.rst correctly, I would say we
should not.
Thanks,
Siddh
On Thu, Apr 18, 2024 at 01:06:42PM +0000, Siddh Raman Pant wrote:
> On Thu, Apr 18 2024 at 14:34:57 +0200, [email protected]
> wrote:
> > On Thu, Apr 18, 2024 at 11:59:41AM +0000, Siddh Raman Pant wrote:
> > > Hi Greg,
> > >
> > > > In the Linux kernel, the following vulnerability has been resolved:
> > > >
> > > > tracing/trigger: Fix to return error if failed to alloc snapshot
> > > >
> > > > Fix register_snapshot_trigger() to return error code if it failed to
> > > > allocate a snapshot instead of 0 (success). Unless that, it will register
> > > > snapshot trigger without an error.
> > >
> > > This commit is problematic on 4.19.y, 5.4.y, 5.10.y, and 5.15.y,
> > > and should be reversed, and this CVE should be rejected for those
> > > versions.
> >
> > Then please submit a patch for this.
>
> Sure.
>
>
> > But note, CVEs are not for specific versions, sorry. We give a hint as
> > to what kernel versions might be affected, but we don not assign CVE to
> > versions.
>
> Cool.
>
> > >
> > > The return value should be 0 on failure, because in the functions
> > > event_trigger_callback() and event_enable_trigger_func(), we have:
> > >
> > > ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> > > /*
> > > * The above returns on success the # of functions enabled,
> > > * but if it didn't find any functions it returns zero.
> > > * Consider no functions a failure too.
> > > */
> > > if (!ret) {
> > > ret = -ENOENT;
> > >
> > > Thus, the commit breaks this assumption.
> > >
> > > This commit needs b8cc44a4d3c1 ("tracing: Remove logic for registering
> > > multiple event triggers at a time") as a prerequisite, as it removes
> > > the above.
> >
> > Should we just take that patch instead?
>
> The series in which the patch is posted is here:
> - https://lore.kernel.org/lkml/[email protected]/
> - https://lore.kernel.org/lkml/[email protected]/
>
> Seems like some good tracing subsystem refactoring. So if I understand
> Documentation/process/stable-kernel-rules.rst correctly, I would say we
> should not.
So the documentation on the commit here is wrong (i.e. wrong Fixes:
tag?) If so, that needs to be said somewhere...
thanks,
greg k-h
On Thu, Apr 18 2024 at 15:13:40 +0200, [email protected]
wrote:
> > > Should we just take that patch instead?
> >
> > The series in which the patch is posted is here:
> > [...]
> >
> > Seems like some good tracing subsystem refactoring. So if I understand
> > Documentation/process/stable-kernel-rules.rst correctly, I would say we
> > should not.
>
> So the documentation on the commit here is wrong (i.e. wrong Fixes:
> tag?) If so, that needs to be said somewhere...
Yes, the Fixes tag seems to be wrong.
Thanks,
Siddh