Re: calcurse-4.3.0p1v0 SIGBUS in pthread_cancel at lib/librthread/rthread.c

+cc maintainer

On 2019/02/01 20:44, Mikolaj Kucharski wrote:
> Hi,
>
> Wanted to try calcurse and at very first usage got crash with SIGBUS. I
> got very easy repro case. I get crash every time by following below
> steps:
>
> - rm -rf ~/.calcurse # that may not be even needed
> - calcurse
> - Press ENTER to continue
> - Press ? for help
> - While being in less (intro.txt) press q to exit the pager
> - Bus error (core dumped)
>
> I didn't contact upstream yet, just wanted to report this here first.
> Does it look to you guys like problem on the calcurse side or
> librthread? I will look to report this upstream too, when time permits.

It feels like bugs on the calcurse side to me.

First problem - program calls notify_start_main_thread to start the
notify thread. First thing this does is try to cancel any existing
notify thread by calling notify_stop_main_thread - on an OS where
pthread_t is just an identifier this would be a noop because there's a
"is not null" check, but on OpenBSD pthread_t is a struct so this is
never null, so it always tries to stop a (nonexistent) thread at first.
This doesn't cause a crash but it is wrong to try to stop a thread that
hasn't been started yet.

Second problem - when you read help, first it stops the thread, then
it calls notify_start_main_thread to try to start it again, but again
(and this time I believe it will be the same on other OS too) it
tries to pthread_cancel/pthread_join the thread which it already
stopped. And this is where it goes boom on OpenBSD because pthread_cancel
tries to dereference a pointer that has already been freed.

Not sure if this is a *good* diff but it does avoid the crash and
I don't think it's too terrible.

Any comments/OKs?


Index: Makefile
===================================================================
RCS file: /cvs/ports/productivity/calcurse/Makefile,v
retrieving revision 1.26
diff -u -p -r1.26 Makefile
--- Makefile 19 Oct 2018 14:19:59 -0000 1.26
+++ Makefile 1 Feb 2019 23:37:48 -0000
@@ -3,7 +3,7 @@
COMMENT= text-based calendar and scheduling application

DISTNAME= calcurse-4.3.0
-REVISION= 1
+REVISION= 2
EPOCH= 0

CATEGORIES= productivity
Index: patches/patch-src_notify_c
===================================================================
RCS file: patches/patch-src_notify_c
diff -N patches/patch-src_notify_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-src_notify_c 1 Feb 2019 23:37:48 -0000
@@ -0,0 +1,46 @@
+$OpenBSD$
+
+Don't assume pthread_t is just an identifier, on OpenBSD it is a struct
+so the "if (notify_t_main)" check is bogus.
+
+Avoid attempting to cancel the same thread multiple times.
+
+Index: src/notify.c
+--- src/notify.c.orig
++++ src/notify.c
+@@ -54,7 +54,7 @@ struct notify_vars {
+ static struct notify_vars notify;
+ static struct notify_app notify_app;
+ static pthread_attr_t detached_thread_attr;
+-static pthread_t notify_t_main;
++static pthread_t *notify_t_main;
+
+ /*
+ * Return the number of seconds before next appointment
+@@ -191,8 +191,10 @@ void notify_free_app(void)
+ void notify_stop_main_thread(void)
+ {
+ if (notify_t_main) {
+- pthread_cancel(notify_t_main);
+- pthread_join(notify_t_main, NULL);
++ pthread_cancel(*notify_t_main);
++ pthread_join(*notify_t_main, NULL);
++ free(notify_t_main);
++ notify_t_main = NULL;
+ }
+ }
+
+@@ -552,7 +554,12 @@ void notify_start_main_thread(void)
+ /* Avoid starting the notification bar thread twice. */
+ notify_stop_main_thread();
+
+- pthread_create(&notify_t_main, NULL, notify_main_thread, NULL);
++ if (notify_t_main == NULL) {
++ if ((notify_t_main = calloc(sizeof notify_t_main, 1)) == NULL)
++ ERROR_MSG(_("calloc"));
++ };
++
++ pthread_create(notify_t_main, NULL, notify_main_thread, NULL);
+ notify_check_next_app(0);
+ }
+