Message ID | b97964653d02225f061e0c2a650b365c354b98c8.1712900945.git.viresh.kumar@linaro.org |
---|---|
State | Accepted |
Commit | b8f85833c05730d631576008daaa34096bc7f3ce |
Headers | show |
Series | cpufreq: exit() callback is optional | expand |
On 12-04-24, 14:19, lizhe wrote:
> Why did you do that? Why did you plagiarize others' achievements? Is this the style of Linaro?
Even if your changes make sense, the discussions needs to be healthy. I am a
co-maintainer of the cpufreq subsystem and its mine and Rafael's responsibility
to keep things moving in the right direction.
This patch fixes a issue you never mentioned over LKML.
Lets not make this awkward, it won't help anyone.
Thanks.
Getting the Cc list back, + Greg. Greg, Looks like another one of those experiments with the community ? :) On 12-04-24, 14:27, lizhe wrote: > You are really disgusting and have no manners at all. This makes people feel disgusted with your company. > > > > ---- Replied Message ---- > | From | Viresh Kumar<viresh.kumar@linaro.org> | > | Date | 04/12/2024 14:24 | > | To | lizhe<sensor1010@163.com> | > | Cc | rafael<rafael@kernel.org>、linux-pm<linux-pm@vger.kernel.org>、Vincent Guittot<vincent.guittot@linaro.org>、linux-kernel<linux-kernel@vger.kernel.org> | > | Subject | Re: [PATCH] cpufreq: exit() callback is optional | > On 12-04-24, 14:12, lizhe wrote: > > I was the first one to find this problem, so the patch should be submitted by me. > > :) > > This patch doesn't take away any of the work you have done. What you are trying > to do is simplify drivers with empty exit callback and the unused return value > of the callback. > > And what I am trying to do is fix a bug in the cpufreq core, which only makes > your other patches more acceptable. > > So no, you never identified the problem this patch is trying to solve. > > Please don't feel that anyone is trying to take away your hardwork. That's not > how things are done here. We appreciate anyone who is spending time to make the > kernel better. > > If I were to take credit of your work, then I would have sent a big patch to fix > the exit() callback issue you are trying to solve, with randomly sent patches.
On 12-04-24, 17:05, lizhe wrote:
> I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch.
Well, I decided not to reply to your emails anymore but this needs to
be clarified a bit now.
You sent a lot of patches, over and over again and it was a mess. I
saw the this [1] series first and went over to read the code and fixed
an issue which I found (by the $subject patch).
Later I read your other patch [2], which I Acked roughly two hours
back and yes you did send a patch that fixed the problem partially. I
never saw it earlier, which is okay and it happens. Despite me giving
an Ack to your patch, you have sent half-a-dozen more emails..
Then I posted a newer version of my patch some time back, removing the
bits you already fixed [3].
That is all one side of the story. But all the noise you have created
here has really demotivated people to review your stuff now.
--
Viresh
[1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/
[2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/
[3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/
On Fri, Apr 12, 2024 at 7:49 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > The exit() callback is optional and shouldn't be called without checking > a valid pointer first. > > Also, we must clear freq_table pointer even if the exit() callback isn't > present. > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> > --- > drivers/cpufreq/cpufreq.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c > index 66e10a19d76a..fd9c3ed21f49 100644 > --- a/drivers/cpufreq/cpufreq.c > +++ b/drivers/cpufreq/cpufreq.c > @@ -1679,10 +1679,13 @@ static void __cpufreq_offline(unsigned int cpu, struct cpufreq_policy *policy) > */ > if (cpufreq_driver->offline) { > cpufreq_driver->offline(policy); > - } else if (cpufreq_driver->exit) { > - cpufreq_driver->exit(policy); > - policy->freq_table = NULL; > + return; > } > + > + if (cpufreq_driver->exit) > + cpufreq_driver->exit(policy); > + > + policy->freq_table = NULL; > } > > static int cpufreq_offline(unsigned int cpu) > @@ -1740,7 +1743,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) > } > > /* We did light-weight exit earlier, do full tear down now */ > - if (cpufreq_driver->offline) > + if (cpufreq_driver->offline && cpufreq_driver->exit) > cpufreq_driver->exit(policy); > > up_write(&policy->rwsem); > -- I have applied this patch (for 6.10 because I don't think it is urgent) because it addresses both issues with missing ->exit() driver callback checks. I honestly don't think it would be better to apply a separate patch for each of them. Thanks!
HI i will review how it all started. 1. i reported the vulnerability to the maintainer. 2. manitaner replay me 3. i report to main line 4. you reply me 5 . you report to main line You submitted the patch to the mainline after you learned about the vulnerability from the patch I submitted At 2024-04-12 21:54:00, "lizhe" <sensor1010@163.com> wrote: Hi Viresh Kumar please add my name to the signature. because i discovered this vulnerability. please reply me. thanks At 2024-04-12 18:28:56, "Greg KH" <gregkh@linuxfoundation.org> wrote: >On Fri, Apr 12, 2024 at 02:51:08PM +0530, Viresh Kumar wrote: >> On 12-04-24, 17:05, lizhe wrote: >> > I found it first and submitted it to the main line first. Please be fair and just. Let him withdraw his patch. >> >> Well, I decided not to reply to your emails anymore but this needs to >> be clarified a bit now. >> >> You sent a lot of patches, over and over again and it was a mess. I >> saw the this [1] series first and went over to read the code and fixed >> an issue which I found (by the $subject patch). >> >> Later I read your other patch [2], which I Acked roughly two hours >> back and yes you did send a patch that fixed the problem partially. I >> never saw it earlier, which is okay and it happens. Despite me giving >> an Ack to your patch, you have sent half-a-dozen more emails.. >> >> Then I posted a newer version of my patch some time back, removing the >> bits you already fixed [3]. >> >> That is all one side of the story. But all the noise you have created >> here has really demotivated people to review your stuff now. >> >> -- >> Viresh >> >> [1] https://lore.kernel.org/all/20240410132132.3526-1-sensor1010@163.com/ >> [2] https://lore.kernel.org/all/20240411231818.2471-1-sensor1010@163.com/ >> [3] https://lore.kernel.org/all/68294ce534668c6ab3b71a1b3e6650227c6e1f20.1712911186.git.viresh.kumar@linaro.org/ > >Thanks for the links, I don't see that you did anything wrong here at >all. > >Lizhe, you seem to be confused as to how kernel development works. I >suggest you take some time off and read up on how this all is supposed >to happen and then work with some local people, in person, to get this >figured out first, before submitting changes again. > >thanks, > >greg k-h
Hi Lizhe, On 4/12/24 08:41, lizhe wrote: > > Let's both take a step back and add my signature to the patch, > since I was the one who discovered and reported the vulnerability. > > You might have discovered the vulnerability and sent in a patch. After that, it is totally up to the maintainer to decide to accept or reject the patch. Developers can't demand their patches to be reviewed and/or accepted. They can request a review and inclusion if maintainer deems it acceptable. In this email thread, I can see that maintainers and developers have been advising you to understand the kernel development process. Refer to the following document to understand the process: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst Refer to the following Contributor Covenant Code of Conduct to understand the code of conduct the Linux kernel community abides by: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst thanks, -- Shuah (On behalf of the Code of Conduct Committee)
On 4/22/24 05:33, lizhe wrote: > The maintainer's obvious robbery behavior. > I submitted patches to the main branch. After receiving them, the maintainer submitted patches again, and the patches contained the patches I submitted. > That is to say, the patches submitted by the maintainer included mine. It is the maintainer who failed to follow the rules. > What's wrong with me submitting patches to the main branch according to the rules! > This is the last email I will be sending to you. It doesn't appear you are willing to engage with the kernel community in a productive and constructive manner. > Refer to the following document to understand the process: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst> > > Refer to the following Contributor Covenant Code of Conduct to understand the > code of conduct the Linux kernel community abides by: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/code-of-conduct.rst> > -- thanks, Shuah (On behalf of the Code of Conduct Committee)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 66e10a19d76a..fd9c3ed21f49 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1679,10 +1679,13 @@ static void __cpufreq_offline(unsigned int cpu, struct cpufreq_policy *policy) */ if (cpufreq_driver->offline) { cpufreq_driver->offline(policy); - } else if (cpufreq_driver->exit) { - cpufreq_driver->exit(policy); - policy->freq_table = NULL; + return; } + + if (cpufreq_driver->exit) + cpufreq_driver->exit(policy); + + policy->freq_table = NULL; } static int cpufreq_offline(unsigned int cpu) @@ -1740,7 +1743,7 @@ static void cpufreq_remove_dev(struct device *dev, struct subsys_interface *sif) } /* We did light-weight exit earlier, do full tear down now */ - if (cpufreq_driver->offline) + if (cpufreq_driver->offline && cpufreq_driver->exit) cpufreq_driver->exit(policy); up_write(&policy->rwsem);
The exit() callback is optional and shouldn't be called without checking a valid pointer first. Also, we must clear freq_table pointer even if the exit() callback isn't present. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- drivers/cpufreq/cpufreq.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)