Bug 6031 - lib/messages_local.c:message_notify can send SIGUSR1 to non Samba processes
lib/messages_local.c:message_notify can send SIGUSR1 to non Samba processes
Status: RESOLVED WONTFIX
Product: Samba 3.2
Classification: Unclassified
Component: File services
unspecified
Other Linux
: P3 normal
: ---
Assigned To: Jeremy Allison
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-01-13 15:36 UTC by Richard Sharpe
Modified: 2012-05-10 08:50 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Sharpe 2009-01-13 15:36:45 UTC
We have encountered cases where smbcontrol has killed vital system processes because it seems that connections.tdb can contain PIDs that are no longer associated with Samba processes.

These vital system processes were coded to panic when receiving an unexpected SIGUSR1.

We have solved this by having message_notify check that the PID is actually one of our processes (smbd, nmbd, winbindd).

Would this change be of interest in general?
Comment 1 Jeremy Allison 2009-01-13 16:06:03 UTC
Sure, I'd like to see the diff, thanks !
Jeremy.
Comment 2 Richard Sharpe 2009-01-13 16:38:03 UTC
Here's the patch for 3.0.x based code, but it shifts to messages_local.c in 3.2.

--- /tmp/tmp.2821.0     2009-01-13 14:35:02.000000000 -0800
+++ /data/prod/main/platform/user/samba/source/lib/messages.c   2008-12-23 10:08:54.000000000 -0800
@@ -164,6 +164,38 @@ static TDB_DATA message_key_pid(struct p
        return kbuf;
 }

+#ifdef $NEWGIG
+static BOOL is_cifs_pid(pid_t pid)
+{
+    fstring ppath, buf = "";
+    FILE *fp;
+    BOOL ret = False;
+
+    fstr_sprintf(ppath, "/proc/%d/cmdline", pid);
+    fp = fopen(ppath, "r");
+    if (!fp) {
+        DEBUG(0, ("Non-existing pid [%d], ignoring without sending\n", pid));
+        return False;
+    }
+    fgets(buf, sizeof(buf), fp);
+    buf[sizeof(buf) - 1] = '\0';
+    if (strlen(buf) && (
+        strstr(buf, "smbd") ||
+        strstr(buf, "winbindd") ||
+        strstr(buf, "nmbd"))) {
+        ret = True;
+    }
+    fclose(fp);
+
+    if (ret == False) {
+        DEBUG(0, ("Non-CIFS pid [%d] cmd [%s], ignoring without sending\n",
+            pid, buf));
+    }
+    return ret;
+}
+#endif
+
+
 /****************************************************************************
  Notify a process that it has a message. If the process doesn't exist
  then delete its record in the database.
@@ -188,6 +220,13 @@ static NTSTATUS message_notify(struct pr
                set_effective_uid(0);
        }

+#ifdef $NEWGIG
+    /* check if we send signal to cifs pid */
+    if (!is_cifs_pid(pid)) {
+        return NT_STATUS_INVALID_HANDLE;
+    }
+#endif
+
        ret = kill(pid, SIGUSR1);

        if (euid != 0) {
Comment 3 Jeremy Allison 2009-01-13 17:00:56 UTC
Ah, a bit too Linux specific for upstream :-(. Although I'll think about this..
Jeremy.
Comment 4 Volker Lendecke 2009-01-14 03:11:52 UTC
Maybe we should finally move to socket-based messaging?

Volker
Comment 5 Richard Sharpe 2012-05-10 08:50:51 UTC
This is so old that it is undoubtedly irrelevant.