setting string session variable registered via component service crashes server

[tomas-villagesql]

MySQL version / commit

any from 9.0 (tested on trunk)

Steps to reproduce

create test file in stock mysql:

--source ../include/have_component_test_session_var_service.inc
INSTALL COMPONENT "file://component_test_session_var_service";
SET SESSION test_component.str_session_var_default = "session_value";
<--- SEGV

Expected vs actual result

not crashing :-)

Platform / compiler

No response

[vsmurthys]

This is an bug we noticed, while this feature was introduced, I have commented out this bug in test case in the file mysql-test/suite/service_sys_var_registration/t/session_var_service.test
I have a fix for it now, attaching the patch.

---

diff --git a/mysql-test/suite/service_sys_var_registration/r/session_var_service.result b/mysql-test/suite/service_sys_var_registration/r/session_var_service.result
index 6bfdeca4125..08c09c307ae 100644
--- a/mysql-test/suite/service_sys_var_registration/r/session_var_service.result
+++ b/mysql-test/suite/service_sys_var_registration/r/session_var_service.result
@@ -101,6 +101,18 @@ SET SESSION test_component.str_session_var=default;
 SELECT @@test_component.str_session_var;
 @@test_component.str_session_var
 NULL
+# Testing string type variable with default value
+SELECT @@test_component.str_session_var_default;
+@@test_component.str_session_var_default
+default
+SET SESSION test_component.str_session_var_default="dictionary.txt";
+SELECT @@test_component.str_session_var_default;
+@@test_component.str_session_var_default
+dictionary.txt
+SET SESSION test_component.str_session_var_default=something;
+SELECT @@test_component.str_session_var_default;
+@@test_component.str_session_var_default
+something
 # Testing unsigned integer type variable
 SELECT @@test_component.uint_session_var;
 @@test_component.uint_session_var
diff --git a/mysql-test/suite/service_sys_var_registration/t/session_var_service.test b/mysql-test/suite/service_sys_var_registration/t/session_var_service.test
index 7c5dc64c446..f5349ab9f28 100644
--- a/mysql-test/suite/service_sys_var_registration/t/session_var_service.test
+++ b/mysql-test/suite/service_sys_var_registration/t/session_var_service.test
@@ -53,12 +53,12 @@ SELECT @@test_component.str_session_var;
 SET SESSION test_component.str_session_var=default;
 SELECT @@test_component.str_session_var;
 
-#--echo # Testing string type variable with default value
-#SELECT @@test_component.str_session_var_default;
-#SET SESSION test_component.str_session_var_default="dictionary.txt";
-#SELECT @@test_component.str_session_var_default;
-#SET SESSION test_component.str_session_var_default=something;
-#SELECT @@test_component.str_session_var_default;
+--echo # Testing string type variable with default value
+SELECT @@test_component.str_session_var_default;
+SET SESSION test_component.str_session_var_default="dictionary.txt";
+SELECT @@test_component.str_session_var_default;
+SET SESSION test_component.str_session_var_default=something;
+SELECT @@test_component.str_session_var_default;
 
 --echo # Testing unsigned integer type variable
 SELECT @@test_component.uint_session_var;
diff --git a/sql/sql_plugin_var.cc b/sql/sql_plugin_var.cc
index ee720c6c8d9..331237355f1 100644
--- a/sql/sql_plugin_var.cc
+++ b/sql/sql_plugin_var.cc
@@ -140,7 +140,22 @@ bool plugin_var_memalloc_session_update(THD *thd, SYS_VAR *var, char **dest,
         list_add(vars->dynamic_variables_allocs, element);
   }
 
-  if (*dest) old_element = (LIST *)(*dest - sizeof(LIST));
+  /*
+    A THD-local MEMALLOC string initially inherits the global value.  That
+    value is not necessarily allocated by this function (a component may,
+    for example, provide a string literal as its default), so it must not be
+    treated as a LIST allocation.  Find an existing allocation in the THD's
+    ownership list before removing it.
+  */
+  if (*dest) {
+    for (LIST *element = vars->dynamic_variables_allocs; element;
+         element = element->next) {
+      if (static_cast<void *>(element + 1) == *dest) {
+        old_element = element;
+        break;
+      }
+    }
+  }
 
   if (var)
     var->update(thd, var, (void **)dest, (const void *)&value);

---

[harinvadodaria]

@tomas-villagesql Thanks for raising the issue.
@vsmurthys Since you already have the patch, May I request you to sign OCA and create a PR? We can then merge it as a contribution.