[PATCH] [v3] ieee802154: ca8210: Fix a potential UAF in ca8210_probe
If of_clk_add_provider() fails in ca8210_register_ext_clock(),
it calls clk_unregister() to release priv->clk and returns an
error. However, the caller ca8210_probe() then calls ca8210_remove(),
where priv->clk is freed again in ca8210_unregister_ext_clock(). In
this case, a use-after-free may happen in the second time we call
clk_unregister().
Fix this by removing the first clk_unregister(). Also, priv->clk could
be an error code on failure of clk_register_fixed_rate(). Use
IS_ERR_OR_NULL to catch this case in ca8210_unregister_ext_clock().
Fixes: ded845a781a5 ("ieee802154: Add CA8210 IEEE 802.15.4 device driver")
Signed-off-by: Dinghao Liu
---
Changelog:
v2: -Remove the first clk_unregister() instead of nulling priv->clk.
v3: -Simplify ca8210_register_ext_clock().
-Add a ';' after return in ca8210_unregister_ext_clock().
---
drivers/net/ieee802154/ca8210.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index aebb19f1b3a4..ae44a9133937 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -2757,18 +2757,8 @@ static int ca8210_register_ext_clock(struct spi_device
*spi)
dev_crit(>dev, "Failed to register external clk\n");
return PTR_ERR(priv->clk);
}
- ret = of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
- if (ret) {
- clk_unregister(priv->clk);
- dev_crit(
- >dev,
- "Failed to register external clock as clock provider\n"
- );
- } else {
- dev_info(>dev, "External clock set as clock provider\n");
- }
- return ret;
+ return of_clk_add_provider(np, of_clk_src_simple_get, priv->clk);
}
/**
@@ -2780,8 +2770,8 @@ static void ca8210_unregister_ext_clock(struct spi_device
*spi)
{
struct ca8210_priv *priv = spi_get_drvdata(spi);
- if (!priv->clk)
- return
+ if (IS_ERR_OR_NULL(priv->clk))
+ return;
of_clk_del_provider(spi->dev.of_node);
clk_unregister(priv->clk);
--
2.17.1
Re: [PATCH 09/13] arm64: dts: qcom: msm8916-longcheer-l8150: Add sound and modem
On Tue, Sep 26, 2023 at 08:59:52PM +0200, Konrad Dybcio wrote: > On 26.09.2023 18:51, Stephan Gerhold wrote: > > From: Nikita Travkin > > > > Enable sound and modem for the Longcheer L8150 (e.g. Wileyfox Swift). > e.g. -> i.e., or is that thing sold under many labels? > Yes, "e.g." is indeed correct here. AFAIK the MSM8916-based Android One devices (aka "google-seed") are also based on the Longcheer L8150. They are available under names like "Cherry Mobile One G1", "i-mobile IQ II", and "General Mobile 4G". They are also covered by this device tree. Thanks, Stephan
Re: [PATCH] eventfs: Test for dentries array allocated in eventfs_release()
On Sat, 30 Sep 2023 09:01:06 -0400
Steven Rostedt wrote:
> From: "Steven Rostedt (Google)"
>
> The dcache_dir_open_wrapper() could be called when a dynamic event is
> being deleted leaving a dentry with no children. In this case the
> dlist->dentries array will never be allocated. This needs to be checked
> for in eventfs_release(), otherwise it will trigger a NULL pointer
> dereference.
Looks good to me.
Acked-by: Masami Hiramatsu (Google)
Thank you,
>
> Fixes: ef36b4f92868 ("eventfs: Remember what dentries were created on dir
> open")
> Signed-off-by: Steven Rostedt (Google)
> ---
> fs/tracefs/event_inode.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 5f1714089884..8c8d64e76103 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -421,7 +421,7 @@ static int eventfs_release(struct inode *inode, struct
> file *file)
> if (WARN_ON_ONCE(!dlist))
> return -EINVAL;
>
> - for (i = 0; dlist->dentries[i]; i++) {
> + for (i = 0; dlist->dentries && dlist->dentries[i]; i++) {
> dput(dlist->dentries[i]);
> }
>
> --
> 2.40.1
>
--
Masami Hiramatsu (Google)
Re: [PATCH 1/2] dt-bindings: mfd: qcom,spmi-pmic: Update gpio example
On 29/09/2023 10:17, Luca Weiss wrote:
> As per commit ea25d61b448a ("arm64: dts: qcom: Use plural _gpios node
> label for PMIC gpios") all dts files now use the plural _gpios instead
> of the singular _gpio as label. Update the schema example also to match.
>
> Signed-off-by: Luca Weiss
> ---
> Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> index 55e931ba5b47..e4842e1fbd65 100644
> --- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
> @@ -245,7 +245,7 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
>
> -pmi8998_gpio: gpio@c000 {
> +pmi8998_gpios: gpio@c000
This does no make sense... you update label only here, but not in any
user of it which proves that label is not used. If it is not used, it
should be dropped, not changed...
Best regards,
Krzysztof
Re: [PATCH 1/3] dt-bindings: i2c: qcom-cci: Document SC7280 compatible
On 29/09/2023 10:01, Luca Weiss wrote: > Document the compatible for the CCI block found on SC7280 SoC. > > Signed-off-by: Luca Weiss > --- > Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > index 042d4dc636ee..158588236749 100644 > --- a/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > +++ b/Documentation/devicetree/bindings/i2c/qcom,i2c-cci.yaml > @@ -25,6 +25,7 @@ properties: > >- items: This is not enough. You miss constraining clocks. Best regards, Krzysztof
[PATCH] eventfs: Test for dentries array allocated in eventfs_release()
From: "Steven Rostedt (Google)"
The dcache_dir_open_wrapper() could be called when a dynamic event is
being deleted leaving a dentry with no children. In this case the
dlist->dentries array will never be allocated. This needs to be checked
for in eventfs_release(), otherwise it will trigger a NULL pointer
dereference.
Fixes: ef36b4f92868 ("eventfs: Remember what dentries were created on dir open")
Signed-off-by: Steven Rostedt (Google)
---
fs/tracefs/event_inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 5f1714089884..8c8d64e76103 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -421,7 +421,7 @@ static int eventfs_release(struct inode *inode, struct file
*file)
if (WARN_ON_ONCE(!dlist))
return -EINVAL;
- for (i = 0; dlist->dentries[i]; i++) {
+ for (i = 0; dlist->dentries && dlist->dentries[i]; i++) {
dput(dlist->dentries[i]);
}
--
2.40.1
[ANNOUNCE] 4.19.295-rt129
Hello RT-list! I'm pleased to announce the 4.19.295-rt129 stable release. This is just an update to the v4.19.295 stable release. No RT specific changes. You can get this release via the git tree at: git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git branch: v4.19-rt Head SHA1: b285af6da7f3b111dd0c285492defb01a4dc91ad Or to build 4.19.295-rt129 directly, the following patches should be applied: https://www.kernel.org/pub/linux/kernel/v4.x/linux-4.19.tar.xz https://www.kernel.org/pub/linux/kernel/v4.x/patch-4.19.295.xz https://www.kernel.org/pub/linux/kernel/projects/rt/4.19/older/patch-4.19.295-rt129.patch.xz Signing key fingerprint: 5BF6 7BC5 0826 72CA BB45 ACAE 587C 5ECA 5D0A 306C All keys used for the above files and repositories can be found on the following git repository: git://git.kernel.org/pub/scm/docs/kernel/pgpkeys.git Enjoy! Daniel
Re: [PATCH] tracing/eprobe: drop unneeded breaks
On Fri, 29 Sep 2023 13:37:08 +0200 (CEST)
Julia Lawall wrote:
>
>
> On Fri, 29 Sep 2023, Masami Hiramatsu wrote:
>
> > On Thu, 28 Sep 2023 12:43:34 +0200
> > Julia Lawall wrote:
> >
> > > Drop break after return.
> > >
> >
> > Good catch! This looks good to me.
> >
> > Acked-by: Masami Hiramatsu (Google)
> >
> > And
> >
> > Fixes: 7491e2c44278 ("tracing: Add a probe that attaches to trace events")
>
> Thanks. I didn't include that because it's not a bug. But it does break
> Coccinelle, which is how I noticed it.
OK, I got it. I thought it may cause a compiler warning because the
'break' never be executed. (maybe it is just a flow-control word,
so it may not need to be warned, but a bit storange.)
>
> julia
>
> >
> > > Signed-off-by: Julia Lawall
> > >
> > > ---
> > > kernel/trace/trace_eprobe.c |5 +
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/trace/trace_eprobe.c b/kernel/trace/trace_eprobe.c
> > > index 72714cbf475c..03c851f57969 100644
> > > --- a/kernel/trace/trace_eprobe.c
> > > +++ b/kernel/trace/trace_eprobe.c
> > > @@ -788,12 +788,9 @@ find_and_get_event(const char *system, const char
> > > *event_name)
> > > name = trace_event_name(tp_event);
> > > if (!name || strcmp(event_name, name))
> > > continue;
> > > - if (!trace_event_try_get_ref(tp_event)) {
> > > + if (!trace_event_try_get_ref(tp_event))
> > > return NULL;
> > > - break;
> > > - }
> > > return tp_event;
> > > - break;
> > > }
> > > return NULL;
> > > }
> > >
> >
> >
> > --
> > Masami Hiramatsu (Google)
> >
--
Masami Hiramatsu (Google)
Re: [PATCH v5 00/12] tracing: fprobe: rethook: Use ftrace_regs instead of pt_regs
On Fri, 29 Sep 2023 17:12:07 -0700 Alexei Starovoitov wrote: > On Thu, Sep 28, 2023 at 6:21 PM Masami Hiramatsu wrote: > > > > > > Thus, what I need is to make fprobe to use function-graph tracer's shadow > > stack and trampoline instead of rethook. This may need to generalize its > > interface so that we can share it between fprobe and function-graph tracer, > > but we don't need to involve rethook and kretprobes anymore. > > ... > > > And need to add patches > > > > - Introduce a generized function exit hook interface for ftrace. > > - Replace rethook in fprobe with the function exit hook interface. > > you mean that rethook will be removed after that? No, it is too late. rethook is deeply integrated with kretprobe. So when we remove the kretprobe, rethook will be removed too. (fprobe and kretprobe provides similar functionality, so we can move to fprobe) Even though, objpool(*) itself might be kept for some other use cases. As far as I can see, ftrace_ret_stack can not provide a context local storage between entry -> exit callbacks. (so this feature must be dropped from fprobe) (*) https://lore.kernel.org/all/20230905015255.81545-1-wuqiang.m...@bytedance.com/ Thank you, -- Masami Hiramatsu (Google)
Re: [PATCH] trace: tracing_event_filter: fast path when no subsystem filters
On Tue, 26 Sep 2023 10:20:58 -0400
Nicholas Lowell wrote:
> From: Nicholas Lowell
>
> If there are no filters in the event subsystem, then there's no
> reason to continue and hit the potentially time consuming
> tracepoint_synchronize_unregister function. This should give
> a speed up for initial disabling/configuring
>
> Signed-off-by: Nicholas Lowell
> ---
> kernel/trace/trace_events_filter.c | 19 ++-
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/trace_events_filter.c
> b/kernel/trace/trace_events_filter.c
> index 33264e510d16..93653d37a132 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -1317,22 +1317,29 @@ void free_event_filter(struct event_filter *filter)
> __free_filter(filter);
> }
>
> -static inline void __remove_filter(struct trace_event_file *file)
> +static inline int __remove_filter(struct trace_event_file *file)
> {
> filter_disable(file);
> - remove_filter_string(file->filter);
> + if (file->filter)
> + remove_filter_string(file->filter);
> + else
> + return 0;
> +
> + return 1;
The above looks awkward. What about:
if (!file->filter)
return 0;
remove_filter_string(file->filter);
return 1;
?
Or better yet:
if (!file->filter)
return false;
remove_filter_string(file->filter);
return true;
and ...
> }
>
> -static void filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> +static int filter_free_subsystem_preds(struct trace_subsystem_dir *dir,
> struct trace_array *tr)
> {
> struct trace_event_file *file;
> + int i = 0;
We don't really need a counter. It's either do the synchronization or
we don't.
bool do_sync = false;
>
> list_for_each_entry(file, >events, list) {
> if (file->system != dir)
> continue;
> - __remove_filter(file);
> + i += __remove_filter(file);
if (remove_filter(file))
do_sync = true;
> }
return do_sync;
> + return i;
> }
>
> static inline void __free_subsystem_filter(struct trace_event_file *file)
> @@ -2411,7 +2418,9 @@ int apply_subsystem_event_filter(struct
> trace_subsystem_dir *dir,
> }
>
> if (!strcmp(strstrip(filter_string), "0")) {
> - filter_free_subsystem_preds(dir, tr);
> + if (filter_free_subsystem_preds(dir, tr) == 0)
> + goto out_unlock;
> +
/* If nothing was freed, we do not need to sync */
if (!filter_free_subsystem_preds(dir, tr))
goto out_unlock;
And yes, add the comment.
And actually, in that block with the goto out_unlock, we should have:
if (!filter_free_subsystem_preds(dir, tr)) {
if (!(WARN_ON_ONCE(system->filter))
goto out_unlock;
}
If there were no preds, ideally there would be no subsystem filter. But
if that's not the case, we need to warn about that and then continue.
-- Steve
> remove_filter_string(system->filter);
> filter = system->filter;
> system->filter = NULL;
Re: [PATCH] ring-buffer: Update "shortest_full" in polling
On Fri, 29 Sep 2023, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)"
>
> It was discovered that the ring buffer polling was incorrectly stating
> that read would not block, but that's because polling did not take into
> account that reads will block if the "buffer-percent" was set. Instead,
> the ring buffer polling would say reads would not block if there was any
> data in the ring buffer. This was incorrect behavior from a user space
> point of view. This was fixed by commit 42fb0a1e84ff by having the polling
> code check if the ring buffer had more data than what the user specified
> "buffer percent" had.
>
> The problem now is that the polling code did not register itself to the
> writer that it wanted to wait for a specific "full" value of the ring
> buffer. The result was that the writer would wake the polling waiter
> whenever there was a new event. The polling waiter would then wake up, see
> that there's not enough data in the ring buffer to notify user space and
> then go back to sleep. The next event would wake it up again.
>
> Before the polling fix was added, the code would wake up around 100 times
> for a hackbench 30 benchmark. After the "fix", due to the constant waking
> of the writer, it would wake up over 11, times! It would never leave
> the kernel, so the user space behavior was still "correct", but this
> definitely is not the desired effect.
>
> To fix this, have the polling code add what it's waiting for to the
> "shortest_full" variable, to tell the writer not to wake it up if the
> buffer is not as full as it expects to be.
>
> Note, after this fix, it appears that the waiter is now woken up around 2x
> the times it was before (~200). This is a tremendous improvement from the
> 11,000 times, but I will need to spend some time to see why polling is
> more aggressive in its wakeups than the read blocking code.
Actually, in my test it has gone from 276 wakeups in 6.0 to only 3 with
this patch.
I can do some more tests.
>
> Cc: sta...@vger.kernel.org
> Fixes: 42fb0a1e84ff ("tracing/ring-buffer: Have polling block on watermark")
> Reported-by: Julia Lawall
> Signed-off-by: Steven Rostedt (Google)
Tested-by: Julia Lawall
julia
> ---
> kernel/trace/ring_buffer.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 28daf0ce95c5..515cafdb18d9 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1137,6 +1137,9 @@ __poll_t ring_buffer_poll_wait(struct trace_buffer
> *buffer, int cpu,
> if (full) {
> poll_wait(filp, >full_waiters, poll_table);
> work->full_waiters_pending = true;
> + if (!cpu_buffer->shortest_full ||
> + cpu_buffer->shortest_full > full)
> + cpu_buffer->shortest_full = full;
> } else {
> poll_wait(filp, >waiters, poll_table);
> work->waiters_pending = true;
> --
> 2.40.1
>
>
