From e42e3aa642b78752ade558412c33985deb8d932e Mon Sep 17 00:00:00 2001
From: Petro Karashchenko <petro.karashchenko@gmail.com>
Date: Sun, 12 Jun 2022 21:14:10 +0200
Subject: [PATCH] drivers/syslog: fix deadlock by reverting part of the changes
 from b88a8cf39ff1019ad787c4316b22ce29c7daa2dc

Signed-off-by: Petro Karashchenko <petro.karashchenko@gmail.com>
---
 drivers/syslog/syslog_device.c | 79 ++++++++++++++++++++++++++++++----
 1 file changed, 70 insertions(+), 9 deletions(-)

diff --git a/drivers/syslog/syslog_device.c b/drivers/syslog/syslog_device.c
index ea744232f5..51d441c923 100644
--- a/drivers/syslog/syslog_device.c
+++ b/drivers/syslog/syslog_device.c
@@ -38,7 +38,7 @@
 #include <nuttx/arch.h>
 #include <nuttx/kmalloc.h>
 #include <nuttx/fs/fs.h>
-#include <nuttx/mutex.h>
+#include <nuttx/semaphore.h>
 #include <nuttx/syslog/syslog.h>
 #include <nuttx/compiler.h>
 
@@ -54,8 +54,10 @@
  */
 
 #define SYSLOG_OFLAGS (O_WRONLY | O_CREAT | O_APPEND)
-#define syslog_dev_takesem(s) nxrmutex_lock(&(s)->sl_lock)
-#define syslog_dev_givesem(s) nxrmutex_unlock(&(s)->sl_lock)
+
+/* An invalid thread ID */
+
+#define NO_HOLDER     (INVALID_PROCESS_ID)
 
 /****************************************************************************
  * Private Types
@@ -81,7 +83,8 @@ struct syslog_dev_s
   uint8_t      sl_state;    /* See enum syslog_dev_state */
   uint8_t      sl_oflags;   /* Saved open mode (for re-open) */
   uint16_t     sl_mode;     /* Saved open flags (for re-open) */
-  rmutex_t     sl_lock;     /* Enforces mutually exclusive access */
+  sem_t        sl_sem;      /* Enforces mutually exclusive access */
+  pid_t        sl_holder;   /* PID of the thread that holds the semaphore */
   struct file  sl_file;     /* The syslog file structure */
   FAR char    *sl_devpath;  /* Full path to the character device */
 };
@@ -120,6 +123,63 @@ static const uint8_t g_syscrlf[2] =
  * Private Functions
  ****************************************************************************/
 
+/****************************************************************************
+ * Name: syslog_dev_takesem
+ ****************************************************************************/
+
+static inline int syslog_dev_takesem(FAR struct syslog_dev_s *syslog_dev)
+{
+  pid_t me = getpid();
+  int ret;
+
+  /* Does this thread already hold the semaphore?  That could happen if
+   * we were called recursively, i.e., if the logic kicked off by
+   * file_write() where to generate more debug output.  Return an
+   * error in that case.
+   */
+
+  if (syslog_dev->sl_holder == me)
+    {
+      /* Return an error (instead of deadlocking) */
+
+      return -EWOULDBLOCK;
+    }
+
+  /* Either the semaphore is available or is currently held by another
+   * thread.  Wait for it to become available.
+   */
+
+  ret = nxsem_wait(&syslog_dev->sl_sem);
+  if (ret < 0)
+    {
+      return ret;
+    }
+
+  /* We hold the semaphore.  We can safely mark ourself as the holder
+   * of the semaphore.
+   */
+
+  syslog_dev->sl_holder = me;
+  return OK;
+}
+
+/****************************************************************************
+ * Name: syslog_dev_givesem
+ ****************************************************************************/
+
+static inline void syslog_dev_givesem(FAR struct syslog_dev_s *syslog_dev)
+{
+#ifdef CONFIG_DEBUG_ASSERTIONS
+  pid_t me = getpid();
+  DEBUGASSERT(syslog_dev->sl_holder == me);
+#endif
+
+  /* Relinquish the semaphore */
+
+  syslog_dev->sl_holder = NO_HOLDER;
+  nxsem_post(&syslog_dev->sl_sem);
+}
+
 /****************************************************************************
  * Name: syslog_dev_open
  *
@@ -212,8 +272,9 @@ static int syslog_dev_open(FAR struct syslog_dev_s *syslog_dev,
 
   /* The SYSLOG device is open and ready for writing. */
 
-  nxrmutex_init(&syslog_dev->sl_lock);
-  syslog_dev->sl_state = SYSLOG_OPENED;
+  nxsem_init(&syslog_dev->sl_sem, 0, 1);
+  syslog_dev->sl_holder = NO_HOLDER;
+  syslog_dev->sl_state  = SYSLOG_OPENED;
   return OK;
 }
 
@@ -242,7 +303,7 @@ static int syslog_dev_open(FAR struct syslog_dev_s *syslog_dev,
  *     close the device, and set it for later re-opening.
  *
  * NOTE: That the third case is different.  It applies only to the thread
- * that currently holds the sl_lock.  Other threads should wait.
+ * that currently holds the sl_sem semaphore.  Other threads should wait.
  * that is why that case is handled in syslog_semtake().
  *
  * Input Parameters:
@@ -291,7 +352,7 @@ static int syslog_dev_outputready(FAR struct syslog_dev_s *syslog_dev)
       if (syslog_dev->sl_state == SYSLOG_FAILURE)
         {
           file_close(&syslog_dev->sl_file);
-          nxrmutex_destroy(&syslog_dev->sl_lock);
+          nxsem_destroy(&syslog_dev->sl_sem);
 
           syslog_dev->sl_state = SYSLOG_REOPEN;
         }
@@ -734,7 +795,7 @@ void syslog_dev_uninitialize(FAR struct syslog_channel_s *channel)
       syslog_dev->sl_state == SYSLOG_FAILURE)
     {
       file_close(&syslog_dev->sl_file);
-      nxrmutex_destroy(&syslog_dev->sl_lock);
+      nxsem_destroy(&syslog_dev->sl_sem);
     }
 
   /* Set the device in UNINITIALIZED state. */