| Message ID | 20230615101656.1308942-4-bigeasy@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: vfp: Introduce vfp_lock() for VFP locking. | expand |
On Thu, 15 Jun 2023 at 12:17, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > vfp_entry() is invoked from exception handler and is fully preemptible. > It uses local_bh_disable() to remain uninterrupted while checking the > VFP state. > This is not working on PREEMPT_RT because local_bh_disable() > synchronizes the relevant section but the context remains fully > preemptible. > > Use vfp_lock() for uninterrupted access. > > VFP_bounce() is invoked from within vfp_entry() and may send a signal. > Sending a signal uses spinlock_t which becomes a sleeping lock > on PREEMPT_RT and must not be acquired within a preempt-disabled > section. Move the vfp_raise_sigfpe() block outside of the > preempt-disabled section. > Isn't the latter a separate issue that predates the softirq changes? If so, it should be fixed separately too, and backported to -stable (unless nobody uses out-of-tree PREEMPT_RT with those kernels) > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > --- > arch/arm/vfp/vfpmodule.c | 33 ++++++++++++++++++++------------- > 1 file changed, 20 insertions(+), 13 deletions(-) > > diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c > index 524aec81134ba..67d7042bc3d5c 100644 > --- a/arch/arm/vfp/vfpmodule.c > +++ b/arch/arm/vfp/vfpmodule.c > @@ -267,7 +267,7 @@ static void vfp_panic(char *reason, u32 inst) > /* > * Process bitmask of exception conditions. > */ > -static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_regs *regs) > +static int vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr) > { > int si_code = 0; > > @@ -275,8 +275,7 @@ static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_ > > if (exceptions == VFP_EXCEPTION_ERROR) { > vfp_panic("unhandled bounce", inst); > - vfp_raise_sigfpe(FPE_FLTINV, regs); > - return; > + return FPE_FLTINV; > } > > /* > @@ -304,8 +303,7 @@ static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_ > RAISE(FPSCR_OFC, FPSCR_OFE, FPE_FLTOVF); > RAISE(FPSCR_IOC, FPSCR_IOE, FPE_FLTINV); > > - if (si_code) > - vfp_raise_sigfpe(si_code, regs); > + return si_code; > } > > /* > @@ -351,6 +349,8 @@ static u32 vfp_emulate_instruction(u32 inst, u32 fpscr, struct pt_regs *regs) > static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) > { > u32 fpscr, orig_fpscr, fpsid, exceptions; > + int si_code2 = 0; > + int si_code = 0; > > pr_debug("VFP: bounce: trigger %08x fpexc %08x\n", trigger, fpexc); > > @@ -396,8 +396,8 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) > * unallocated VFP instruction but with FPSCR.IXE set and not > * on VFP subarch 1. > */ > - vfp_raise_exceptions(VFP_EXCEPTION_ERROR, trigger, fpscr, regs); > - return; > + si_code = vfp_raise_exceptions(VFP_EXCEPTION_ERROR, trigger, fpscr); > + goto exit; > } > > /* > @@ -421,14 +421,14 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) > */ > exceptions = vfp_emulate_instruction(trigger, fpscr, regs); > if (exceptions) > - vfp_raise_exceptions(exceptions, trigger, orig_fpscr, regs); > + si_code2 = vfp_raise_exceptions(exceptions, trigger, orig_fpscr); > > /* > * If there isn't a second FP instruction, exit now. Note that > * the FPEXC.FP2V bit is valid only if FPEXC.EX is 1. > */ > if ((fpexc & (FPEXC_EX | FPEXC_FP2V)) != (FPEXC_EX | FPEXC_FP2V)) > - return; > + goto exit; > > /* > * The barrier() here prevents fpinst2 being read > @@ -440,7 +440,13 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) > emulate: > exceptions = vfp_emulate_instruction(trigger, orig_fpscr, regs); > if (exceptions) > - vfp_raise_exceptions(exceptions, trigger, orig_fpscr, regs); > + si_code = vfp_raise_exceptions(exceptions, trigger, orig_fpscr); > +exit: > + vfp_unlock(); > + if (si_code2) > + vfp_raise_sigfpe(si_code2, regs); > + if (si_code) > + vfp_raise_sigfpe(si_code, regs); > } > > static void vfp_enable(void *unused) > @@ -707,7 +713,7 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) > if (!user_mode(regs)) > return vfp_kmode_exception(regs, trigger); > > - local_bh_disable(); > + vfp_lock(); > fpexc = fmrx(FPEXC); > > /* > @@ -772,6 +778,7 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) > * replay the instruction that trapped. > */ > fmxr(FPEXC, fpexc); > + vfp_unlock(); > } else { > /* Check for synchronous or asynchronous exceptions */ > if (!(fpexc & (FPEXC_EX | FPEXC_DEX))) { > @@ -786,17 +793,17 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) > if (!(fpscr & FPSCR_IXE)) { > if (!(fpscr & FPSCR_LENGTH_MASK)) { > pr_debug("not VFP\n"); > - local_bh_enable(); > + vfp_unlock(); > return -ENOEXEC; > } > fpexc |= FPEXC_DEX; > } > } > bounce: regs->ARM_pc += 4; > + /* VFP_bounce() will invoke vfp_unlock() */ > VFP_bounce(trigger, fpexc, regs); > } > > - local_bh_enable(); > return 0; > } > > -- > 2.40.1 >
On 2023-06-27 12:22:23 [+0200], Ard Biesheuvel wrote: > On Thu, 15 Jun 2023 at 12:17, Sebastian Andrzej Siewior > <bigeasy@linutronix.de> wrote: > > > > vfp_entry() is invoked from exception handler and is fully preemptible. > > It uses local_bh_disable() to remain uninterrupted while checking the > > VFP state. > > This is not working on PREEMPT_RT because local_bh_disable() > > synchronizes the relevant section but the context remains fully > > preemptible. > > > > Use vfp_lock() for uninterrupted access. > > > > VFP_bounce() is invoked from within vfp_entry() and may send a signal. > > Sending a signal uses spinlock_t which becomes a sleeping lock > > on PREEMPT_RT and must not be acquired within a preempt-disabled > > section. Move the vfp_raise_sigfpe() block outside of the > > preempt-disabled section. > > > > Isn't the latter a separate issue that predates the softirq changes? > If so, it should be fixed separately too, and backported to -stable > (unless nobody uses out-of-tree PREEMPT_RT with those kernels) This only affects the PREEMPT_RT tree and once this is settled I would ask to backport the PREEMPT_RT related fixes within the preempt-rt stable trees and not bother the official stable trees. Thanks for noting ;) Sebastian
On Tue, 27 Jun 2023 at 14:41, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2023-06-27 12:22:23 [+0200], Ard Biesheuvel wrote: > > On Thu, 15 Jun 2023 at 12:17, Sebastian Andrzej Siewior > > <bigeasy@linutronix.de> wrote: > > > > > > vfp_entry() is invoked from exception handler and is fully preemptible. > > > It uses local_bh_disable() to remain uninterrupted while checking the > > > VFP state. > > > This is not working on PREEMPT_RT because local_bh_disable() > > > synchronizes the relevant section but the context remains fully > > > preemptible. > > > > > > Use vfp_lock() for uninterrupted access. > > > > > > VFP_bounce() is invoked from within vfp_entry() and may send a signal. > > > Sending a signal uses spinlock_t which becomes a sleeping lock > > > on PREEMPT_RT and must not be acquired within a preempt-disabled > > > section. Move the vfp_raise_sigfpe() block outside of the > > > preempt-disabled section. > > > > > > > Isn't the latter a separate issue that predates the softirq changes? > > If so, it should be fixed separately too, and backported to -stable > > (unless nobody uses out-of-tree PREEMPT_RT with those kernels) > > This only affects the PREEMPT_RT tree and once this is settled I would > ask to backport the PREEMPT_RT related fixes within the preempt-rt > stable trees and not bother the official stable trees. > > Thanks for noting ;) > OK, so would it make sense to split this off into a separate patch, and combine all the vfp_lock/unlock replacements into a second patch. Note that Russell didn't pull my VFP work so it won't be landing in v6.5.
On 2023-06-27 14:57:38 [+0200], Ard Biesheuvel wrote: > > OK, so would it make sense to split this off into a separate patch, > and combine all the vfp_lock/unlock replacements into a second patch. If you want I can split it. Given that the code in v6.1 looks different, I'm not sure if I can apply it as-it or need to adapt it anyway. > Note that Russell didn't pull my VFP work so it won't be landing in v6.5. Good, or not. Please keep me in the loop if you repost so I can rebase accordingly if needed. Sebastian
On Tue, Jun 27, 2023 at 03:06:02PM +0200, Sebastian Andrzej Siewior wrote: > On 2023-06-27 14:57:38 [+0200], Ard Biesheuvel wrote: > > > > OK, so would it make sense to split this off into a separate patch, > > and combine all the vfp_lock/unlock replacements into a second patch. > > If you want I can split it. Given that the code in v6.1 looks different, > I'm not sure if I can apply it as-it or need to adapt it anyway. > > > Note that Russell didn't pull my VFP work so it won't be landing in v6.5. > Good, or not. Please keep me in the loop if you repost so I can rebase > accordingly if needed. Probably a good thing if the problem you have is caused by Ard's series... as the patch system is down for the long term until Debian fix their mariadb security regression which prevents a server requiring a minimum of TLS v1.2 accepting connections from Debian Bookworm systems (which regress from supporting TLS v1.3 to a maximum of TLS v1.1). I reported this as a bug to the debian BTS last week, and as I have come to expect from useless waste of space distro bug tracking systems, there has been zero reaction - and as the bug has already been reported by others, and they've been fobbed off by the package maintainer, I am not hopeful that this regression will ever be fixed. It seems to me that Debian is ripe for having a CVE raised against Bookworm for the regression, especially as TLS v1.1 is now regarded as vulnerable due to downgrade attacks - and maybe that will make Debian sit up and take notice.
On Tue, 27 Jun 2023 at 15:22, Russell King (Oracle) <linux@armlinux.org.uk> wrote: > > On Tue, Jun 27, 2023 at 03:06:02PM +0200, Sebastian Andrzej Siewior wrote: > > On 2023-06-27 14:57:38 [+0200], Ard Biesheuvel wrote: > > > > > > OK, so would it make sense to split this off into a separate patch, > > > and combine all the vfp_lock/unlock replacements into a second patch. > > > > If you want I can split it. Given that the code in v6.1 looks different, > > I'm not sure if I can apply it as-it or need to adapt it anyway. > > > > > Note that Russell didn't pull my VFP work so it won't be landing in v6.5. > > Good, or not. Please keep me in the loop if you repost so I can rebase > > accordingly if needed. > > Probably a good thing if the problem you have is caused by Ard's > series... No, they are independent. Note that PREEMPT_RT for ARM is not in mainline, so this is not a regression of any kind for mainline itself. I suggested to Sebastian to rebase his changes on top of my PR, but that seems to have fallen between the cracks. > as the patch system is down for the long term until Debian > fix their mariadb security regression which prevents a server requiring > a minimum of TLS v1.2 accepting connections from Debian Bookworm > systems (which regress from supporting TLS v1.3 to a maximum of TLS > v1.1). > > I reported this as a bug to the debian BTS last week, and as I have > come to expect from useless waste of space distro bug tracking systems, > there has been zero reaction - and as the bug has already been reported > by others, and they've been fobbed off by the package maintainer, I am > not hopeful that this regression will ever be fixed. > > It seems to me that Debian is ripe for having a CVE raised against > Bookworm for the regression, especially as TLS v1.1 is now regarded > as vulnerable due to downgrade attacks - and maybe that will make > Debian sit up and take notice. > Does this mean the ARM tree is closed for business a the moment? Is there no way to carry on without the patch system?
On Tue, Jun 27, 2023 at 03:26:16PM +0200, Ard Biesheuvel wrote: > On Tue, 27 Jun 2023 at 15:22, Russell King (Oracle) > <linux@armlinux.org.uk> wrote: > > as the patch system is down for the long term until Debian > > fix their mariadb security regression which prevents a server requiring > > a minimum of TLS v1.2 accepting connections from Debian Bookworm > > systems (which regress from supporting TLS v1.3 to a maximum of TLS > > v1.1). > > > > I reported this as a bug to the debian BTS last week, and as I have > > come to expect from useless waste of space distro bug tracking systems, > > there has been zero reaction - and as the bug has already been reported > > by others, and they've been fobbed off by the package maintainer, I am > > not hopeful that this regression will ever be fixed. > > > > It seems to me that Debian is ripe for having a CVE raised against > > Bookworm for the regression, especially as TLS v1.1 is now regarded > > as vulnerable due to downgrade attacks - and maybe that will make > > Debian sit up and take notice. > > > > Does this mean the ARM tree is closed for business a the moment? Is > there no way to carry on without the patch system? Only if you want to see just how utterly terrible I am trying to pick patches off the mailing list... IMHO, the better thing would be to apply pressure to Debian to get them to at least recognise their cockup, so that then we have some idea when stuff can be resurected.
On 2023-06-27 14:22:36 [+0100], Russell King (Oracle) wrote: > Probably a good thing if the problem you have is caused by Ard's > series... as the patch system is down for the long term until Debian Ard's previous series (reworking the VFP/softirq and allowing VFP usage from within softirq) broke things for RT or made me aware of another problem. The vfp_bounce() problem was unkown. This series of Ard is completely independent and makes it easier to fix it since the code isn't part in .S and part .c so it made things easier. > fix their mariadb security regression which prevents a server requiring > a minimum of TLS v1.2 accepting connections from Debian Bookworm > systems (which regress from supporting TLS v1.3 to a maximum of TLS > v1.1). > > I reported this as a bug to the debian BTS last week, and as I have > come to expect from useless waste of space distro bug tracking systems, > there has been zero reaction - and as the bug has already been reported > by others, and they've been fobbed off by the package maintainer, I am > not hopeful that this regression will ever be fixed. Do you have a BTS number, I may try to ping folks. > It seems to me that Debian is ripe for having a CVE raised against > Bookworm for the regression, especially as TLS v1.1 is now regarded > as vulnerable due to downgrade attacks - and maybe that will make > Debian sit up and take notice. Sebastian
On Tue, Jun 27, 2023 at 02:32:39PM +0100, Russell King (Oracle) wrote: > On Tue, Jun 27, 2023 at 03:26:16PM +0200, Ard Biesheuvel wrote: > > On Tue, 27 Jun 2023 at 15:22, Russell King (Oracle) > > <linux@armlinux.org.uk> wrote: > > > as the patch system is down for the long term until Debian > > > fix their mariadb security regression which prevents a server requiring > > > a minimum of TLS v1.2 accepting connections from Debian Bookworm > > > systems (which regress from supporting TLS v1.3 to a maximum of TLS > > > v1.1). > > > > > > I reported this as a bug to the debian BTS last week, and as I have > > > come to expect from useless waste of space distro bug tracking systems, > > > there has been zero reaction - and as the bug has already been reported > > > by others, and they've been fobbed off by the package maintainer, I am > > > not hopeful that this regression will ever be fixed. > > > > > > It seems to me that Debian is ripe for having a CVE raised against > > > Bookworm for the regression, especially as TLS v1.1 is now regarded > > > as vulnerable due to downgrade attacks - and maybe that will make > > > Debian sit up and take notice. > > > > > > > Does this mean the ARM tree is closed for business a the moment? Is > > there no way to carry on without the patch system? > > Only if you want to see just how utterly terrible I am trying to pick > patches off the mailing list... > > IMHO, the better thing would be to apply pressure to Debian to get > them to at least recognise their cockup, so that then we have some > idea when stuff can be resurected. Doing a bit more digging, it seems my /usr/bin/mysql was leading me somewhat astray - it was mariadb-client-core-10.1 despite following the recommended debian upgrade path over the years - so that dates from a time when TLS v1.1 was the max. However, that's just a distraction. libdbd-mysql-perl refuses to even attempt to connect - it seems to bail out before even trying. The confusing thing is that /usr/bin/mysql was reporting error 2026, and when one traces libdbd-mysql-perl, the trace file also reports error 2026: imp_dbh->bind_type_guessing: 0 imp_dbh->use_server_side_prepare: 0 imp_dbh->disable_fallback_for_server_prepare: 0 --> do_error SSL connection error: Enforcing SSL encryption is not supported error 2026 recorded: SSL connection error: Enforcing SSL encryption is not supported <-- do_error <> DESTROY(DBI::db=HASH(0x255d118)) ignored for outer handle (inner DBI::db=HASH(0x256ca80) has ref cnt 2) However, these two error numbers, although identical, are totally different. I haven't yet figured out where "error 2026" comes from in libdbd-mysql-perl. _This_ 2026 error comes from libdbd-mysql-perl itself, and after quite a bit of research, it's due to: if (ssl_enforce) { <== true #if defined(HAVE_SSL_MODE_ONLY_REQUIRED) <== not defined ... #elif defined(HAVE_SSL_ENFORCE) <== not defined ... #elif defined(HAVE_SSL_VERIFY) <== defined if (!ssl_verify_also_enforce_ssl()) { set_ssl_error(sock, "Enforcing SSL encryption is not supported"); return NULL; } ssl_verify_also_enforce_ssl() does this: static inline bool ssl_verify_also_enforce_ssl(void) { #ifdef MARIADB_BASE_VERSION my_ulonglong version = mysql_get_client_version(); return ((version >= 50544 && version < 50600) || (version >= 100020 && version < 100100) || version >= 100106); #else return false; #endif } and mocking up a program to print the result from on Bookworm, mysql_get_client_version() gives: Version = 30305 Where does that come from... #define MARIADB_CLIENT_VERSION_STR "10.11.3" #define MARIADB_BASE_VERSION "mariadb-10.11" #define MARIADB_VERSION_ID 101103 #define MYSQL_CONFIG_NAME "my" #define MYSQL_VERSION_ID 101103 #define MYSQL_SERVER_VERSION "10.11.3-MariaDB" #define MARIADB_PACKAGE_VERSION "3.3.5" #define MARIADB_PACKAGE_VERSION_ID 30305 So, mysql_get_client_version() for mariadb reports the mariadb _package_ version, not the mysql version. The same under Bullseye: Version = 100519 #define MARIADB_CLIENT_VERSION_STR "10.5.19" #define MARIADB_BASE_VERSION "mariadb-10.5" #define MARIADB_VERSION_ID 100519 #define MYSQL_CONFIG_NAME "my" #define MYSQL_VERSION_ID 100519 #define MYSQL_SERVER_VERSION "10.5.19-MariaDB" #define MARIADB_PACKAGE_VERSION "3.1.20" #define MARIADB_PACKAGE_VERSION_ID 30120 So, libdbd-mysql-perl believes that mariadb 10.11.3 is ancient compared to mariadb 10.5.19. I think I need to update that bug with this new information - and it points to a mariadb problem, specifically with libmariadb, and not a TLS regression after all.
On Tue, Jun 27, 2023 at 03:40:10PM +0100, Russell King (Oracle) wrote: > On Tue, Jun 27, 2023 at 02:32:39PM +0100, Russell King (Oracle) wrote: > > On Tue, Jun 27, 2023 at 03:26:16PM +0200, Ard Biesheuvel wrote: > > > On Tue, 27 Jun 2023 at 15:22, Russell King (Oracle) > > > <linux@armlinux.org.uk> wrote: > > > > as the patch system is down for the long term until Debian > > > > fix their mariadb security regression which prevents a server requiring > > > > a minimum of TLS v1.2 accepting connections from Debian Bookworm > > > > systems (which regress from supporting TLS v1.3 to a maximum of TLS > > > > v1.1). > > > > > > > > I reported this as a bug to the debian BTS last week, and as I have > > > > come to expect from useless waste of space distro bug tracking systems, > > > > there has been zero reaction - and as the bug has already been reported > > > > by others, and they've been fobbed off by the package maintainer, I am > > > > not hopeful that this regression will ever be fixed. > > > > > > > > It seems to me that Debian is ripe for having a CVE raised against > > > > Bookworm for the regression, especially as TLS v1.1 is now regarded > > > > as vulnerable due to downgrade attacks - and maybe that will make > > > > Debian sit up and take notice. > > > > > > > > > > Does this mean the ARM tree is closed for business a the moment? Is > > > there no way to carry on without the patch system? > > > > Only if you want to see just how utterly terrible I am trying to pick > > patches off the mailing list... > > > > IMHO, the better thing would be to apply pressure to Debian to get > > them to at least recognise their cockup, so that then we have some > > idea when stuff can be resurected. > > Doing a bit more digging, it seems my /usr/bin/mysql was leading me > somewhat astray - it was mariadb-client-core-10.1 despite following > the recommended debian upgrade path over the years - so that dates > from a time when TLS v1.1 was the max. > > However, that's just a distraction. libdbd-mysql-perl refuses to > even attempt to connect - it seems to bail out before even trying. > > The confusing thing is that /usr/bin/mysql was reporting error 2026, > and when one traces libdbd-mysql-perl, the trace file also reports > error 2026: > > imp_dbh->bind_type_guessing: 0 > imp_dbh->use_server_side_prepare: 0 > imp_dbh->disable_fallback_for_server_prepare: 0 > --> do_error > SSL connection error: Enforcing SSL encryption is not supported error 2026 recorded: SSL connection error: Enforcing SSL encryption is not supported > <-- do_error > <> DESTROY(DBI::db=HASH(0x255d118)) ignored for outer handle (inner DBI::db=HASH(0x256ca80) has ref cnt 2) > > However, these two error numbers, although identical, are totally > different. I haven't yet figured out where "error 2026" comes from > in libdbd-mysql-perl. > > _This_ 2026 error comes from libdbd-mysql-perl itself, and after > quite a bit of research, it's due to: > > if (ssl_enforce) { <== true > #if defined(HAVE_SSL_MODE_ONLY_REQUIRED) <== not defined > ... > #elif defined(HAVE_SSL_ENFORCE) <== not defined > ... > #elif defined(HAVE_SSL_VERIFY) <== defined > if (!ssl_verify_also_enforce_ssl()) { > set_ssl_error(sock, "Enforcing SSL encryption is not supported"); > return NULL; > } > > ssl_verify_also_enforce_ssl() does this: > > static inline bool ssl_verify_also_enforce_ssl(void) { > #ifdef MARIADB_BASE_VERSION > my_ulonglong version = mysql_get_client_version(); > return ((version >= 50544 && version < 50600) || (version >= 100020 && version < 100100) || version >= 100106); > #else > return false; > #endif > } > > and mocking up a program to print the result from on Bookworm, > mysql_get_client_version() gives: > > Version = 30305 > > Where does that come from... > > #define MARIADB_CLIENT_VERSION_STR "10.11.3" > #define MARIADB_BASE_VERSION "mariadb-10.11" > #define MARIADB_VERSION_ID 101103 > #define MYSQL_CONFIG_NAME "my" > #define MYSQL_VERSION_ID 101103 > #define MYSQL_SERVER_VERSION "10.11.3-MariaDB" > #define MARIADB_PACKAGE_VERSION "3.3.5" > #define MARIADB_PACKAGE_VERSION_ID 30305 > > So, mysql_get_client_version() for mariadb reports the mariadb _package_ > version, not the mysql version. > > The same under Bullseye: > > Version = 100519 > > #define MARIADB_CLIENT_VERSION_STR "10.5.19" > #define MARIADB_BASE_VERSION "mariadb-10.5" > #define MARIADB_VERSION_ID 100519 > #define MYSQL_CONFIG_NAME "my" > #define MYSQL_VERSION_ID 100519 > #define MYSQL_SERVER_VERSION "10.5.19-MariaDB" > #define MARIADB_PACKAGE_VERSION "3.1.20" > #define MARIADB_PACKAGE_VERSION_ID 30120 > > So, libdbd-mysql-perl believes that mariadb 10.11.3 is ancient compared > to mariadb 10.5.19. > > I think I need to update that bug with this new information - and it > points to a mariadb problem, specifically with libmariadb, and not a > TLS regression after all. And doing more digging... https://github.com/mariadb-corporation/mariadb-connector-c/commit/a37b7c3965706f9a062baaba0c494dd6efb2c306 So, a change in what the mariadb developers thought was a good idea to report though this interface breaks the perl DBD library... and given that it effectively winds-back the value returned from mysql_get_client_version(), who knows if there's now any sane way of testing the numerical result of that function.
On Tue, Jun 27, 2023 at 04:02:04PM +0100, Russell King (Oracle) wrote: > On Tue, Jun 27, 2023 at 03:40:10PM +0100, Russell King (Oracle) wrote: > > On Tue, Jun 27, 2023 at 02:32:39PM +0100, Russell King (Oracle) wrote: > > > On Tue, Jun 27, 2023 at 03:26:16PM +0200, Ard Biesheuvel wrote: > > > > On Tue, 27 Jun 2023 at 15:22, Russell King (Oracle) > > > > <linux@armlinux.org.uk> wrote: > > > > > as the patch system is down for the long term until Debian > > > > > fix their mariadb security regression which prevents a server requiring > > > > > a minimum of TLS v1.2 accepting connections from Debian Bookworm > > > > > systems (which regress from supporting TLS v1.3 to a maximum of TLS > > > > > v1.1). > > > > > > > > > > I reported this as a bug to the debian BTS last week, and as I have > > > > > come to expect from useless waste of space distro bug tracking systems, > > > > > there has been zero reaction - and as the bug has already been reported > > > > > by others, and they've been fobbed off by the package maintainer, I am > > > > > not hopeful that this regression will ever be fixed. > > > > > > > > > > It seems to me that Debian is ripe for having a CVE raised against > > > > > Bookworm for the regression, especially as TLS v1.1 is now regarded > > > > > as vulnerable due to downgrade attacks - and maybe that will make > > > > > Debian sit up and take notice. > > > > > > > > > > > > > Does this mean the ARM tree is closed for business a the moment? Is > > > > there no way to carry on without the patch system? > > > > > > Only if you want to see just how utterly terrible I am trying to pick > > > patches off the mailing list... > > > > > > IMHO, the better thing would be to apply pressure to Debian to get > > > them to at least recognise their cockup, so that then we have some > > > idea when stuff can be resurected. > > > > Doing a bit more digging, it seems my /usr/bin/mysql was leading me > > somewhat astray - it was mariadb-client-core-10.1 despite following > > the recommended debian upgrade path over the years - so that dates > > from a time when TLS v1.1 was the max. > > > > However, that's just a distraction. libdbd-mysql-perl refuses to > > even attempt to connect - it seems to bail out before even trying. > > > > The confusing thing is that /usr/bin/mysql was reporting error 2026, > > and when one traces libdbd-mysql-perl, the trace file also reports > > error 2026: > > > > imp_dbh->bind_type_guessing: 0 > > imp_dbh->use_server_side_prepare: 0 > > imp_dbh->disable_fallback_for_server_prepare: 0 > > --> do_error > > SSL connection error: Enforcing SSL encryption is not supported error 2026 recorded: SSL connection error: Enforcing SSL encryption is not supported > > <-- do_error > > <> DESTROY(DBI::db=HASH(0x255d118)) ignored for outer handle (inner DBI::db=HASH(0x256ca80) has ref cnt 2) > > > > However, these two error numbers, although identical, are totally > > different. I haven't yet figured out where "error 2026" comes from > > in libdbd-mysql-perl. > > > > _This_ 2026 error comes from libdbd-mysql-perl itself, and after > > quite a bit of research, it's due to: > > > > if (ssl_enforce) { <== true > > #if defined(HAVE_SSL_MODE_ONLY_REQUIRED) <== not defined > > ... > > #elif defined(HAVE_SSL_ENFORCE) <== not defined > > ... > > #elif defined(HAVE_SSL_VERIFY) <== defined > > if (!ssl_verify_also_enforce_ssl()) { > > set_ssl_error(sock, "Enforcing SSL encryption is not supported"); > > return NULL; > > } > > > > ssl_verify_also_enforce_ssl() does this: > > > > static inline bool ssl_verify_also_enforce_ssl(void) { > > #ifdef MARIADB_BASE_VERSION > > my_ulonglong version = mysql_get_client_version(); > > return ((version >= 50544 && version < 50600) || (version >= 100020 && version < 100100) || version >= 100106); > > #else > > return false; > > #endif > > } > > > > and mocking up a program to print the result from on Bookworm, > > mysql_get_client_version() gives: > > > > Version = 30305 > > > > Where does that come from... > > > > #define MARIADB_CLIENT_VERSION_STR "10.11.3" > > #define MARIADB_BASE_VERSION "mariadb-10.11" > > #define MARIADB_VERSION_ID 101103 > > #define MYSQL_CONFIG_NAME "my" > > #define MYSQL_VERSION_ID 101103 > > #define MYSQL_SERVER_VERSION "10.11.3-MariaDB" > > #define MARIADB_PACKAGE_VERSION "3.3.5" > > #define MARIADB_PACKAGE_VERSION_ID 30305 > > > > So, mysql_get_client_version() for mariadb reports the mariadb _package_ > > version, not the mysql version. > > > > The same under Bullseye: > > > > Version = 100519 > > > > #define MARIADB_CLIENT_VERSION_STR "10.5.19" > > #define MARIADB_BASE_VERSION "mariadb-10.5" > > #define MARIADB_VERSION_ID 100519 > > #define MYSQL_CONFIG_NAME "my" > > #define MYSQL_VERSION_ID 100519 > > #define MYSQL_SERVER_VERSION "10.5.19-MariaDB" > > #define MARIADB_PACKAGE_VERSION "3.1.20" > > #define MARIADB_PACKAGE_VERSION_ID 30120 > > > > So, libdbd-mysql-perl believes that mariadb 10.11.3 is ancient compared > > to mariadb 10.5.19. > > > > I think I need to update that bug with this new information - and it > > points to a mariadb problem, specifically with libmariadb, and not a > > TLS regression after all. > > And doing more digging... > > https://github.com/mariadb-corporation/mariadb-connector-c/commit/a37b7c3965706f9a062baaba0c494dd6efb2c306 > > So, a change in what the mariadb developers thought was a good idea > to report though this interface breaks the perl DBD library... and > given that it effectively winds-back the value returned from > mysql_get_client_version(), who knows if there's now any sane way of > testing the numerical result of that function. Oh, and this just keeps getting better. Read the entire thread on: https://github.com/mariadb-corporation/mariadb-connector-c/pull/219 Whoever thought it was a good idea that mysql_get_client_version() should suddenly be wound back to a smaller number... clearly wasn't thinking the implications through when users of the library are going to be doing stuff like: https://github.com/perl5-dbi/DBD-mysql/blame/master/dbdimp.h#L107 which is over six years old. It really makes me wonder why we try in the kernel not to regress userspace when userspace people don't themselves seem to "get it" and make idiotic changes like this. Anyway, I've now found what seems to be a workaround - telling perl's DBD driver that SSL is optional turns off enforced SSL, and thus stops the DBD driver checking the library version. Not ideal, but at least it gets stuff working again, while the mariadb people work out how to fix their gigantic cockup.
On 2023-06-27 18:41:32 [+0100], Russell King (Oracle) wrote:
> Oh, and this just keeps getting better.
…
In the meantime Ard's patches got applied to
http://git.armlinux.org.uk/cgit/linux-arm.git/log/?h=devel-stable
and I rebased my series on top
https://git.kernel.org/pub/scm/linux/kernel/git/bigeasy/staging.git/log/?h=arm_vfp_rmk
The patches in this thread do apply.
Is there anything I could do?
Sebastian
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c index 524aec81134ba..67d7042bc3d5c 100644 --- a/arch/arm/vfp/vfpmodule.c +++ b/arch/arm/vfp/vfpmodule.c @@ -267,7 +267,7 @@ static void vfp_panic(char *reason, u32 inst) /* * Process bitmask of exception conditions. */ -static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_regs *regs) +static int vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr) { int si_code = 0; @@ -275,8 +275,7 @@ static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_ if (exceptions == VFP_EXCEPTION_ERROR) { vfp_panic("unhandled bounce", inst); - vfp_raise_sigfpe(FPE_FLTINV, regs); - return; + return FPE_FLTINV; } /* @@ -304,8 +303,7 @@ static void vfp_raise_exceptions(u32 exceptions, u32 inst, u32 fpscr, struct pt_ RAISE(FPSCR_OFC, FPSCR_OFE, FPE_FLTOVF); RAISE(FPSCR_IOC, FPSCR_IOE, FPE_FLTINV); - if (si_code) - vfp_raise_sigfpe(si_code, regs); + return si_code; } /* @@ -351,6 +349,8 @@ static u32 vfp_emulate_instruction(u32 inst, u32 fpscr, struct pt_regs *regs) static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) { u32 fpscr, orig_fpscr, fpsid, exceptions; + int si_code2 = 0; + int si_code = 0; pr_debug("VFP: bounce: trigger %08x fpexc %08x\n", trigger, fpexc); @@ -396,8 +396,8 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) * unallocated VFP instruction but with FPSCR.IXE set and not * on VFP subarch 1. */ - vfp_raise_exceptions(VFP_EXCEPTION_ERROR, trigger, fpscr, regs); - return; + si_code = vfp_raise_exceptions(VFP_EXCEPTION_ERROR, trigger, fpscr); + goto exit; } /* @@ -421,14 +421,14 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) */ exceptions = vfp_emulate_instruction(trigger, fpscr, regs); if (exceptions) - vfp_raise_exceptions(exceptions, trigger, orig_fpscr, regs); + si_code2 = vfp_raise_exceptions(exceptions, trigger, orig_fpscr); /* * If there isn't a second FP instruction, exit now. Note that * the FPEXC.FP2V bit is valid only if FPEXC.EX is 1. */ if ((fpexc & (FPEXC_EX | FPEXC_FP2V)) != (FPEXC_EX | FPEXC_FP2V)) - return; + goto exit; /* * The barrier() here prevents fpinst2 being read @@ -440,7 +440,13 @@ static void VFP_bounce(u32 trigger, u32 fpexc, struct pt_regs *regs) emulate: exceptions = vfp_emulate_instruction(trigger, orig_fpscr, regs); if (exceptions) - vfp_raise_exceptions(exceptions, trigger, orig_fpscr, regs); + si_code = vfp_raise_exceptions(exceptions, trigger, orig_fpscr); +exit: + vfp_unlock(); + if (si_code2) + vfp_raise_sigfpe(si_code2, regs); + if (si_code) + vfp_raise_sigfpe(si_code, regs); } static void vfp_enable(void *unused) @@ -707,7 +713,7 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) if (!user_mode(regs)) return vfp_kmode_exception(regs, trigger); - local_bh_disable(); + vfp_lock(); fpexc = fmrx(FPEXC); /* @@ -772,6 +778,7 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) * replay the instruction that trapped. */ fmxr(FPEXC, fpexc); + vfp_unlock(); } else { /* Check for synchronous or asynchronous exceptions */ if (!(fpexc & (FPEXC_EX | FPEXC_DEX))) { @@ -786,17 +793,17 @@ static int vfp_support_entry(struct pt_regs *regs, u32 trigger) if (!(fpscr & FPSCR_IXE)) { if (!(fpscr & FPSCR_LENGTH_MASK)) { pr_debug("not VFP\n"); - local_bh_enable(); + vfp_unlock(); return -ENOEXEC; } fpexc |= FPEXC_DEX; } } bounce: regs->ARM_pc += 4; + /* VFP_bounce() will invoke vfp_unlock() */ VFP_bounce(trigger, fpexc, regs); } - local_bh_enable(); return 0; }
vfp_entry() is invoked from exception handler and is fully preemptible. It uses local_bh_disable() to remain uninterrupted while checking the VFP state. This is not working on PREEMPT_RT because local_bh_disable() synchronizes the relevant section but the context remains fully preemptible. Use vfp_lock() for uninterrupted access. VFP_bounce() is invoked from within vfp_entry() and may send a signal. Sending a signal uses spinlock_t which becomes a sleeping lock on PREEMPT_RT and must not be acquired within a preempt-disabled section. Move the vfp_raise_sigfpe() block outside of the preempt-disabled section. Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- arch/arm/vfp/vfpmodule.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)