From: Eliav Farber <[email protected]>

commit f8efca92ae509c25e0a4bd5d0a86decea4f0c41e upstream.

Do alignment logic properly and use the "ptr" local variable for
calculating the remainder of the alignment.

This became an issue because struct edac_mc_layer has a size that is not
zero modulo eight, and the next offset that was prepared for the private
data was unaligned, causing an alignment exception.

The patch in Fixes: which broke this actually wanted to "what we
actually care about is the alignment of the actual pointer that's about
to be returned." But it didn't check that alignment.

Use the correct variable "ptr" for that.

[ bp: Massage commit message. ]

Fixes: 8447c4d15e35 ("edac: Do alignment logic properly in edac_align_ptr()")
Signed-off-by: Eliav Farber <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/edac/edac_mc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -265,7 +265,7 @@ void *edac_align_ptr(void **p, unsigned
else
return (char *)ptr;

- r = (unsigned long)p % align;
+ r = (unsigned long)ptr % align;

if (r == 0)
return (char *)ptr;

2022-02-22 05:46:18

Hi!

> syzbot reported that two threads might write over agg_select_timer
> at the same time. Make agg_select_timer atomic to fix the races.

Ok, but:

> --- a/drivers/net/bonding/bond_3ad.c
> +++ b/drivers/net/bonding/bond_3ad.c
> @@ -249,7 +249,7 @@ static inline int __check_agg_selection_
> if (bond == NULL)
> return 0;
>
> - return BOND_AD_INFO(bond).agg_select_timer ? 1 : 0;
> + return atomic_read(&BOND_AD_INFO(bond).agg_select_timer) ? 1 : 0;
> }

This could probably use !!.

> +static bool bond_agg_timer_advance(struct bonding *bond)
> +{
> + int val, nval;
> +
> + while (1) {
> + val = atomic_read(&BOND_AD_INFO(bond).agg_select_timer);
> + if (!val)
> + return false;
> + nval = val - 1;
> + if (atomic_cmpxchg(&BOND_AD_INFO(bond).agg_select_timer,
> + val, nval) == val)
> + break;
> + }
> + return nval == 0;
> +}

This should really be atomic_dec_if_positive, no?

Best regards,
Pavel
--
http://www.livejournal.com/~pavelmachek

Attachments:

(No filename) (1.00 kB)
signature.asc (188.00 B)
Digital signature Download all attachments

2022-02-22 16:45:33

by Pavel Machek

[permalink] [raw]

Subject: Re: [PATCH 4.19 49/58] KVM: x86/pmu: Use AMD64_RAW_EVENT_MASK for PERF_TYPE_RAW

Hi!

> [ Upstream commit 710c476514313c74045c41c0571bb5178fd16e3d ]
>
> AMD's event select is 3 nybbles, with the high nybble in bits 35:32 of
> a PerfEvtSeln MSR. Don't mask off the high nybble when configuring a
> RAW perf event.

Ok, but this depends on b8bfee "KVM: x86/pmu: Don't truncate the
PerfEvtSeln MSR when creating a perf event", and that is backported to
5.10 but not 4.19... so we don't need this, either, right...?

Best regards,
Pavel

> +++ b/arch/x86/kvm/pmu.c
> @@ -171,7 +171,7 @@ void reprogram_gp_counter(struct kvm_pmc *pmc, u64 eventsel)
> }
>
> if (type == PERF_TYPE_RAW)
> - config = eventsel & X86_RAW_EVENT_MASK;
> + config = eventsel & AMD64_RAW_EVENT_MASK;
>
> pmc_reprogram_counter(pmc, type, config,
> !(eventsel & ARCH_PERFMON_EVENTSEL_USR),

--
http://www.livejournal.com/~pavelmachek

Attachments:

(No filename) (880.00 B)
signature.asc (188.00 B)
Digital signature Download all attachments

2022-02-22 18:05:20

by Eric Dumazet

[permalink] [raw]

Subject: Re: [PATCH 4.19 29/58] bonding: fix data-races around agg_select_timer

On Tue, Feb 22, 2022 at 7:35 AM Pavel Machek <[email protected]> wrote:
>
> Hi!
>
> > syzbot reported that two threads might write over agg_select_timer
> > at the same time. Make agg_select_timer atomic to fix the races.
>
> Ok, but:
>
> > --- a/drivers/net/bonding/bond_3ad.c
> > +++ b/drivers/net/bonding/bond_3ad.c
> > @@ -249,7 +249,7 @@ static inline int __check_agg_selection_
> > if (bond == NULL)
> > return 0;
> >
> > - return BOND_AD_INFO(bond).agg_select_timer ? 1 : 0;
> > + return atomic_read(&BOND_AD_INFO(bond).agg_select_timer) ? 1 : 0;
> > }
>
> This could probably use !!.

Probably... I chose to not change code style in a bug fix.

>
> > +static bool bond_agg_timer_advance(struct bonding *bond)
> > +{
> > + int val, nval;
> > +
> > + while (1) {
> > + val = atomic_read(&BOND_AD_INFO(bond).agg_select_timer);
> > + if (!val)
> > + return false;
> > + nval = val - 1;
> > + if (atomic_cmpxchg(&BOND_AD_INFO(bond).agg_select_timer,
> > + val, nval) == val)
> > + break;
> > + }
> > + return nval == 0;
> > +}
>
> This should really be atomic_dec_if_positive, no?

SGTM, please send a patch, thank you.