Re: [PATCH v6] cpufreq/pasemi: fix an use-after-free inpas_cpufreq_cpu_init()
On 16-07-19, 16:26, wen.yan...@zte.com.cn wrote: > Okay thank you. > Now this patch > (https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/) > > seems to have not been merged into the linux-next. > > In order to avoid code conflicts, we will wait until this patch is merged in > and then send v7. Please rebase on PM tree's linux-next branch instead and resend your patch. git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git -- viresh
Re: [PATCH v6] cpufreq/pasemi: fix an use-after-free inpas_cpufreq_cpu_init()
> > The cpu variable is still being used in the of_get_property() call > > after the of_node_put() call, which may result in use-after-free. > > > > Fixes: a9acc26b75f6 ("cpufreq/pasemi: fix possible object reference leak") > > Signed-off-by: Wen Yang > > Cc: "Rafael J. Wysocki" > > Cc: Viresh Kumar > > Cc: Michael Ellerman > > Cc: linuxppc-dev@lists.ozlabs.org > > Cc: linux...@vger.kernel.org > > Cc: linux-ker...@vger.kernel.org > > --- > > v6: keep the blank line and fix warning: label 'out_unmap_sdcpwr' defined > > but not used. > > v5: put together the code to get, use, and release cpu device_node. > > v4: restore the blank line. > > v3: fix a leaked reference. > > v2: clean up the code according to the advice of viresh. > > > > drivers/cpufreq/pasemi-cpufreq.c | 26 ++ > > 1 file changed, 14 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/cpufreq/pasemi-cpufreq.c > > b/drivers/cpufreq/pasemi-cpufreq.c > > index 6b1e4ab..7d557f9 100644 > > --- a/drivers/cpufreq/pasemi-cpufreq.c > > +++ b/drivers/cpufreq/pasemi-cpufreq.c > > @@ -131,10 +131,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > int err = -ENODEV; > > > > cpu = of_get_cpu_node(policy->cpu, NULL); > > +if (!cpu) > > +goto out; > > > > +max_freqp = of_get_property(cpu, "clock-frequency", NULL); > > of_node_put(cpu); > > -if (!cpu) > > +if (!max_freqp) { > > +err = -EINVAL; > > goto out; > > +} > > + > > +/* we need the freq in kHz */ > > +max_freq = *max_freqp / 1000; > > > > dn = of_find_compatible_node(NULL, NULL, "1682m-sdc"); > > if (!dn) > > @@ -171,16 +179,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > } > > > > pr_debug("init cpufreq on CPU %d\n", policy->cpu); > > - > > -max_freqp = of_get_property(cpu, "clock-frequency", NULL); > > -if (!max_freqp) { > > -err = -EINVAL; > > -goto out_unmap_sdcpwr; > > -} > > - > > -/* we need the freq in kHz */ > > -max_freq = *max_freqp / 1000; > > - > > pr_debug("max clock-frequency is at %u kHz\n", max_freq); > > pr_debug("initializing frequency table\n"); > > > > @@ -196,7 +194,11 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > policy->cur = pas_freqs[cur_astate].frequency; > > ppc_proc_freq = policy->cur * 1000ul; > > > > -return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency()); > > +err = cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency()); > > So you are trying to fix an earlier issue here with this. Should have > been a separate patch. Over that I have just sent a patch now to make > this routine return void. > > https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/ > > So all you need to do is to remove the label out_unmap_sdcpwr instead. Okay thank you. Now this patch (https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/) seems to have not been merged into the linux-next. In order to avoid code conflicts, we will wait until this patch is merged in and then send v7. -- Thanks and regards, Wen > > +if (err) > > +goto out_unmap_sdcpwr; > > + > > +return 0; > > > > out_unmap_sdcpwr: > > iounmap(sdcpwr_mapbase); > > -- > > 2.9.5
Re: [PATCH v6] cpufreq/pasemi: fix an use-after-free inpas_cpufreq_cpu_init()
> > The cpu variable is still being used in the of_get_property() call > > after the of_node_put() call, which may result in use-after-free. > > > > Fixes: a9acc26b75f6 ("cpufreq/pasemi: fix possible object reference leak") > > Signed-off-by: Wen Yang > > Cc: "Rafael J. Wysocki" > > Cc: Viresh Kumar > > Cc: Michael Ellerman > > Cc: linuxppc-dev@lists.ozlabs.org > > Cc: linux...@vger.kernel.org > > Cc: linux-ker...@vger.kernel.org > > --- > > v6: keep the blank line and fix warning: label 'out_unmap_sdcpwr' defined > > but not used. > > v5: put together the code to get, use, and release cpu device_node. > > v4: restore the blank line. > > v3: fix a leaked reference. > > v2: clean up the code according to the advice of viresh. > > > > drivers/cpufreq/pasemi-cpufreq.c | 26 ++ > > 1 file changed, 14 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/cpufreq/pasemi-cpufreq.c > > b/drivers/cpufreq/pasemi-cpufreq.c > > index 6b1e4ab..7d557f9 100644 > > --- a/drivers/cpufreq/pasemi-cpufreq.c > > +++ b/drivers/cpufreq/pasemi-cpufreq.c > > @@ -131,10 +131,18 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > int err = -ENODEV; > > > > cpu = of_get_cpu_node(policy->cpu, NULL); > > +if (!cpu) > > +goto out; > > > > +max_freqp = of_get_property(cpu, "clock-frequency", NULL); > > of_node_put(cpu); > > -if (!cpu) > > +if (!max_freqp) { > > +err = -EINVAL; > > goto out; > > +} > > + > > +/* we need the freq in kHz */ > > +max_freq = *max_freqp / 1000; > > > > dn = of_find_compatible_node(NULL, NULL, "1682m-sdc"); > > if (!dn) > > @@ -171,16 +179,6 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > } > > > > pr_debug("init cpufreq on CPU %d\n", policy->cpu); > > - > > -max_freqp = of_get_property(cpu, "clock-frequency", NULL); > > -if (!max_freqp) { > > -err = -EINVAL; > > -goto out_unmap_sdcpwr; > > -} > > - > > -/* we need the freq in kHz */ > > -max_freq = *max_freqp / 1000; > > - > > pr_debug("max clock-frequency is at %u kHz\n", max_freq); > > pr_debug("initializing frequency table\n"); > > > > @@ -196,7 +194,11 @@ static int pas_cpufreq_cpu_init(struct cpufreq_policy > > *policy) > > policy->cur = pas_freqs[cur_astate].frequency; > > ppc_proc_freq = policy->cur * 1000ul; > > > > -return cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency()); > > +err = cpufreq_generic_init(policy, pas_freqs, get_gizmo_latency()); > > So you are trying to fix an earlier issue here with this. Should have > been a separate patch. Over that I have just sent a patch now to make > this routine return void. > > https://lore.kernel.org/lkml/ee8cf5fb4b4a01fdf9199037ff6d835b935cfd13.1562902877.git.viresh.ku...@linaro.org/ > > So all you need to do is to remove the label out_unmap_sdcpwr instead. Ok, thanks. -- Cheers, Wen