| Message ID | 1487588781-15123-3-git-send-email-elena.reshetova@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote: > refcount_t type and corresponding API should be > used instead of atomic_t when the variable is used as > a reference counter. This allows to avoid accidental > refcounter overflows that might lead to use-after-free > situations. > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Kees Cook <keescook@chromium.org> > Signed-off-by: David Windsor <dwindsor@gmail.com> That SOB chain tells me that you wrote the patch and Hans, Kees and David handled it in some way and the last one - David - is sending it to me. It doesn't look like that though. So what are you trying to express with it? Documentation/process/submitting-patches.rst has some info on the whole SOB deal: 11) Sign your work — the Developer's Certificate of Origin ---------------------------------------------------------- ..
> On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote: > > refcount_t type and corresponding API should be > > used instead of atomic_t when the variable is used as > > a reference counter. This allows to avoid accidental > > refcounter overflows that might lead to use-after-free > > situations. > > > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > > Signed-off-by: Kees Cook <keescook@chromium.org> > > Signed-off-by: David Windsor <dwindsor@gmail.com> > > That SOB chain tells me that you wrote the patch and Hans, Kees and > David handled it in some way and the last one - David - is sending it to > me. It doesn't look like that though. > > So what are you trying to express with it? Whole refcount conversion was a long piece of work and the above people contributed to this code either as writes or reviewers or both. I am primary writer of the code and I am handing patches in our tree and sending them out, so how exactly the above should look like? Please note that we have about 300 patches and if I have to modify each sign-off to reflect who contributed to each commit in what particular way, I will go insane. Best Regards, Elena.
On Mon, Feb 20, 2017 at 12:20:04PM +0000, Reshetova, Elena wrote: > Whole refcount conversion was a long piece of work and the above > people contributed to this code either as writes or reviewers or both. We have Reviewed-by: for reviewers. > I am primary writer of the code and I am handing patches in our tree > and sending them out, so how exactly the above should look like? Well, the SOB chain should reflect who handled the patch on its way from the original author to the upstream committer. If you want to express who contributed, you can use Originally-by, Suggested-by, ... You could also have free text in the commit message: "People who contributed to this: ..." In any case, it is all in Documentation/process/submitting-patches.rst. Have a look. > Please note that we have about 300 patches and if I have to modify > each sign-off to reflect who contributed to each commit in what > particular way, I will go insane. There's sed/awk/perl/python/bash... whatever tools that can do the proper conversions with.
On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote: > On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote: >> refcount_t type and corresponding API should be >> used instead of atomic_t when the variable is used as >> a reference counter. This allows to avoid accidental >> refcounter overflows that might lead to use-after-free >> situations. >> >> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> >> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> >> Signed-off-by: Kees Cook <keescook@chromium.org> >> Signed-off-by: David Windsor <dwindsor@gmail.com> > > That SOB chain tells me that you wrote the patch and Hans, Kees and > David handled it in some way and the last one - David - is sending it to > me. It doesn't look like that though. Perhaps the least inaccurate form of this might be: Inspired by atomic protections in PaX/grsecurity. Suggested-by: Kees Cook <keescook@chromium.org> Reviewed-by: David Windsor <dwindsor@gmail.com> Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> As this is something I'd suggested we implement based on the work in PaX/grsecurity, David took the first (and continuing) stab at conversions, Hans did more, and Elena has been doing even more along with the heavy-lifting of keeping the series organized. That way the first SoB is still the author, the last SoB is still the email sender, and everyone's name is mentioned. Or just: Inspired by atomic protections in PaX/grsecurity, based on work from David Windsor, Hans Liljestrand, and myself. Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> I'm not picky -- I just want to see the conversion to refcount_t happen, and everyone in Elena's SoB list has done work on it... -Kees
On Tue, Feb 21, 2017 at 12:45:30PM -0800, Kees Cook wrote: > On Mon, Feb 20, 2017 at 3:17 AM, Borislav Petkov <bp@alien8.de> wrote: > > On Mon, Feb 20, 2017 at 01:06:19PM +0200, Elena Reshetova wrote: > >> refcount_t type and corresponding API should be > >> used instead of atomic_t when the variable is used as > >> a reference counter. This allows to avoid accidental > >> refcounter overflows that might lead to use-after-free > >> situations. > >> > >> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > >> Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com> > >> Signed-off-by: Kees Cook <keescook@chromium.org> > >> Signed-off-by: David Windsor <dwindsor@gmail.com> > > > > That SOB chain tells me that you wrote the patch and Hans, Kees and > > David handled it in some way and the last one - David - is sending it to > > me. It doesn't look like that though. > > Perhaps the least inaccurate form of this might be: > > > Inspired by atomic protections in PaX/grsecurity. > > Suggested-by: Kees Cook <keescook@chromium.org> > Reviewed-by: David Windsor <dwindsor@gmail.com> > Reviewed-by: Hans Liljestrand <ishkamiel@gmail.com> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > > As this is something I'd suggested we implement based on the work in > PaX/grsecurity, David took the first (and continuing) stab at > conversions, Hans did more, and Elena has been doing even more along > with the heavy-lifting of keeping the series organized. That way the > first SoB is still the author, the last SoB is still the email sender, > and everyone's name is mentioned. > > Or just: > > > Inspired by atomic protections in PaX/grsecurity, based on work from > David Windsor, Hans Liljestrand, and myself. > > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com> > > > I'm not picky -- I just want to see the conversion to refcount_t Me neither - both look good to me and actually explain what the SOB chain was trying to say. Thanks!
diff --git a/arch/x86/include/asm/amd_nb.h b/arch/x86/include/asm/amd_nb.h index 00c88a0..da181ad 100644 --- a/arch/x86/include/asm/amd_nb.h +++ b/arch/x86/include/asm/amd_nb.h @@ -3,6 +3,7 @@ #include <linux/ioport.h> #include <linux/pci.h> +#include <linux/refcount.h> struct amd_nb_bus_dev_range { u8 bus; @@ -55,7 +56,7 @@ struct threshold_bank { struct threshold_block *blocks; /* initialized to the number of CPUs on the node sharing this bank */ - atomic_t cpus; + refcount_t cpus; }; struct amd_northbridge { diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c index 524cc57..cfe0838 100644 --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c @@ -1202,7 +1202,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank) goto out; per_cpu(threshold_banks, cpu)[bank] = b; - atomic_inc(&b->cpus); + refcount_inc(&b->cpus); err = __threshold_add_blocks(b); @@ -1225,7 +1225,7 @@ static int threshold_create_bank(unsigned int cpu, unsigned int bank) per_cpu(threshold_banks, cpu)[bank] = b; if (is_shared_bank(bank)) { - atomic_set(&b->cpus, 1); + refcount_set(&b->cpus, 1); /* nb is already initialized, see above */ if (nb) { @@ -1289,7 +1289,7 @@ static void threshold_remove_bank(unsigned int cpu, int bank) goto free_out; if (is_shared_bank(bank)) { - if (!atomic_dec_and_test(&b->cpus)) { + if (!refcount_dec_and_test(&b->cpus)) { __threshold_remove_blocks(b); per_cpu(threshold_banks, cpu)[bank] = NULL; return;