Solved Odd Segfault, Very Specific and Obscure Steps to Reproduce

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

I was wondering if anyone could look at my code and be able to help identify what I'm doing wrong? It gets the current working directory of a given process id using libprocstat. I normally does work and can get the cwd of a pid just fine, but that's assmuing I don't do the specific thing in my program that causes the crash, which is just a specific order of input and instructions with seemingly unrelated code having to do with getting the PEB (process environment block) as a separate task. Here's the line it segfaults at:



I'm wondering if I'm not freeing memory somewhere, or I am not doing it correctly. But from what I read on the libprocstat man pages it looks pretty correct. Unless of course, the filestat * needs to be freed, in which case I couldn't find any documentation on what the best or proper way to do that is. Here's a video showing how to reproduce, although not very helpful because it requires installing a bunch of software I don't expect any of you to install, as it is a real beast of many dependencies and rather annoying setup instructions.

View: https://www.youtube.com/watch?v=d_ctSj1qBiY

I don't even bother to explain what I'm doing that reproduces it in the video, I'm just using that to show what line it segfaults at when run through GDB. Basically, just take my word for it, it has very specific steps to reproduce the segfault and it can't seem to be reproduced any other way in my testing. Thank you for your time. If this isn't showing enough information I don't really have any easy way to show more of my code due to how it is being used. I'll call it quits for getting help here if that happens to be the case.

Samuel
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

WAG: cntp is less than one.
I have it loop back to cntp - 1 when I iterate below zero, and I have it loop back to zero when it is equal to cntp going the other way. Every time I scroll the mouse wheel to change iteration the process id list I have set up to get updated automatically. It only crashes after I trigger that kdialog popup which calls the PEB code to get the environment block. Otherwise it lets me go below zero and loop to the max value no problem.

On FreeBSD 13.0-RELEASE I see already an error when compiling crossprocess.cpp.

git clone https://github.com/time-killer-games/CrossProcess.git crossprocess
cd crossprocess
clang++ -c crossprocess.cpp
Code:
crossprocess.cpp:474:11: error: cannot initialize a variable of type 'char *' with an rvalue of type 'const std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::value_type *' (aka 'const char *')
    char *exe = str1.data();
          ^     ~~~~~~~~~~~
1 error generated.

Then I changed the declaration on line 474 to const char *exe = ... and got just the next error.
Code:
crossprocess.cpp:475:9: error: no matching function for call to 'sysctl'
    if (sysctl(mib, 4, exe, &s, nullptr, 0) == 0) {
        ^~~~~~
/usr/include/sys/sysctl.h:1185:5: note: candidate function not viable: no known conversion from 'const char *' to 'void *' for 3rd argument; take the address of the argument with &
int     sysctl(const int *, u_int, void *, size_t *, const void *, size_t);
        ^
1 error generated.
Now on line 475 I forced the exe parameter to be of type (char *), and then this compiles without errors. But this is not how it should be done. If the returned value of str1.data() really and seriously points to a read only memory block, and then when sysctl() tries to write to this block, that will lead to a crash.

clang++ --analyze -c crossprocess.cpp -o crossprocess.xml
Code:
crossprocess.cpp:633:11: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
      if (cmdline) {
          ^~~~~~~
crossprocess.cpp:732:7: warning: Branch condition evaluates to a garbage value [core.uninitialized.Branch]
  if (buffer) {
      ^~~~~~
2 warnings generated.

The XML file is a bit messy to read, and I prefer to see the results of the static analyzer in an IDE like Xcode. Anyway here it comes. This shows the problematic executions paths step by step giving the line of each step, and the condition which brings you to the next step.

crossprocess.xml
XML:
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>clang_version</key>
<string>FreeBSD clang version 11.0.1 (git@github.com:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c3fe)</string>
<key>diagnostics</key>
<array>
  <dict>
   <key>path</key>
   <array>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>627</integer>
           <key>col</key><integer>3</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>627</integer>
           <key>col</key><integer>24</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>3</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>4</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>3</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>4</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>12</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>pop-up</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>629</integer>
      <key>col</key><integer>7</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>629</integer>
         <key>col</key><integer>7</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>629</integer>
         <key>col</key><integer>12</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>extended_message</key>
     <string>&apos;procId&apos; is non-null</string>
     <key>message</key>
     <string>&apos;procId&apos; is non-null</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>629</integer>
           <key>col</key><integer>12</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>630</integer>
           <key>col</key><integer>5</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>630</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>630</integer>
      <key>col</key><integer>21</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>630</integer>
         <key>col</key><integer>21</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>630</integer>
         <key>col</key><integer>29</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>depth</key><integer>0</integer>
     <key>extended_message</key>
     <string>Entering loop body</string>
     <key>message</key>
     <string>Entering loop body</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>630</integer>
           <key>col</key><integer>5</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>630</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>631</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>631</integer>
           <key>col</key><integer>10</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>631</integer>
      <key>col</key><integer>7</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>631</integer>
         <key>col</key><integer>7</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>631</integer>
         <key>col</key><integer>20</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>depth</key><integer>0</integer>
     <key>extended_message</key>
     <string>&apos;cmdline&apos; declared without an initial value</string>
     <key>message</key>
     <string>&apos;cmdline&apos; declared without an initial value</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>631</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>631</integer>
           <key>col</key><integer>10</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>632</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>632</integer>
           <key>col</key><integer>23</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>632</integer>
      <key>col</key><integer>7</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>632</integer>
         <key>col</key><integer>7</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>632</integer>
         <key>col</key><integer>55</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>depth</key><integer>0</integer>
     <key>extended_message</key>
     <string>Calling &apos;CmdlineFromProcId&apos;</string>
     <key>message</key>
     <string>Calling &apos;CmdlineFromProcId&apos;</string>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>557</integer>
      <key>col</key><integer>1</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>depth</key><integer>1</integer>
     <key>extended_message</key>
     <string>Entered call from &apos;ProcIdFromParentProcIdSkipSh&apos;</string>
     <key>message</key>
     <string>Entered call from &apos;ProcIdFromParentProcIdSkipSh&apos;</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>557</integer>
           <key>col</key><integer>1</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>557</integer>
           <key>col</key><integer>4</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>30</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>35</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>30</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>35</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>30</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>35</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>558</integer>
      <key>col</key><integer>30</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>depth</key><integer>1</integer>
     <key>extended_message</key>
     <string>Returning without writing to &apos;*buffer&apos;</string>
     <key>message</key>
     <string>Returning without writing to &apos;*buffer&apos;</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>30</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>35</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>30</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>558</integer>
           <key>col</key><integer>35</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>632</integer>
      <key>col</key><integer>7</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>632</integer>
         <key>col</key><integer>7</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>632</integer>
         <key>col</key><integer>55</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>depth</key><integer>0</integer>
     <key>extended_message</key>
     <string>Returning from &apos;CmdlineFromProcId&apos;</string>
     <key>message</key>
     <string>Returning from &apos;CmdlineFromProcId&apos;</string>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>632</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>632</integer>
           <key>col</key><integer>23</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>8</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>control</string>
     <key>edges</key>
      <array>
       <dict>
        <key>start</key>
         <array>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>7</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>8</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
        <key>end</key>
         <array>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>11</integer>
           <key>file</key><integer>0</integer>
          </dict>
          <dict>
           <key>line</key><integer>633</integer>
           <key>col</key><integer>17</integer>
           <key>file</key><integer>0</integer>
          </dict>
         </array>
       </dict>
      </array>
    </dict>
    <dict>
     <key>kind</key><string>event</string>
     <key>location</key>
     <dict>
      <key>line</key><integer>633</integer>
      <key>col</key><integer>11</integer>
      <key>file</key><integer>0</integer>
     </dict>
     <key>ranges</key>
     <array>
       <array>
        <dict>
         <key>line</key><integer>633</integer>
         <key>col</key><integer>11</integer>
         <key>file</key><integer>0</integer>
        </dict>
        <dict>
         <key>line</key><integer>633</integer>
         <key>col</key><integer>17</integer>
         <key>file</key><integer>0</integer>
        </dict>
       </array>
     </array>
     <key>depth</key><integer>0</integer>
     <key>extended_message</key>
     <string>Branch condition evaluates to a garbage value</string>
     <key>message</key>
     <string>Branch condition evaluates to a garbage value</string>
    </dict>
   </array>
   <key>description</key><string>Branch condition evaluates to a garbage value</string>
   <key>category</key><string>Logic error</string>
   <key>type</key><string>Branch condition evaluates to a garbage value</string>
   <key>check_name</key><string>core.uninitialized.Branch</string>
   <!-- This hash is experimental and going to change! -->
   <key>issue_hash_content_of_line_in_context</key><string>2fe90dc8eae113ac644d2d178c3bef04</string>
  <key>issue_context_kind</key><string>function</string>
  <key>issue_context</key><string>ProcIdFromParentProcIdSkipSh</string>
  <key>issue_hash_function_offset</key><string>7</string>
...

So, clangs static analyzer points to 2 other potential crashers in your code.
I would recommend compiling with C++17 or similar. Sorry, that's important information I left out.

Edit:

It's worth noting if you compile with C++17 a std::string whatever.data() will be a char * and not a const char * and therefore will compile. I had a question about the rest of your post but I went out to eat fast food. That's why my reply didn't address everything you said initially, and didn't know that would rub you wrong enough to delete your post, sorry I got hungry?

Anyway, I'm a bit curious how those two warnings could possibly cause a crash. cmdline and buffer in some cases won't be initialized in which case they will equal 'garbage' or rather last I checked that just means it will equal a null pointer because of trying to copy to the buffers but it finds no information to copy. If I'm mistaken, would initializing it as a null pointer fix it? I'm going to try that now, but I'm confused why that warning came out for cmdline but not environ (and others) which has the same issue it would seem.

Edit2:

Initializing the buffers as nullptr before filling them didn't solve it. Honestly I still feel a bit lost with this. I thank you for the help you've given thus far, it certainly will serve as a good hint solving the problem if nothing else. That clang command I had no idea existed and I am definitely going to use that going forward with debugging stuff like this here and there.

While it did remove the two warnings, it doesn't solve the crash in this case, because I am not even using either of those two functions, which threw warnings, in my test application, (the one in the video that crashes). Although it is helpful to have those warnings fixed! :D
 

covacat

Well-Known Member

Reaction score: 171
Messages: 365

if procid dies somewhere between the lprocstat calls it segvs
this is a simplified version of the cpp file
Code:
#include <sys/socket.h>
#include <sys/sysctl.h>
#include <sys/param.h>
#include <sys/queue.h>
#include <sys/user.h>
#include <libprocstat.h>
#include <libutil.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char ** argv) {
char cwd[PATH_MAX]; unsigned cntp;
char buf[11];
int procId = atoi(argv[1]);
  printf("%d \n",procId);
  fflush(stdout);
  struct procstat *proc_stat = procstat_open_sysctl();
  printf("PS:%p\n",proc_stat);
  struct kinfo_proc *proc_info = procstat_getprocs(proc_stat, KERN_PROC_PID, procId, &cntp);
  printf("PI:%p\n",proc_info);
 
  if(!proc_info) exit(1); 
  printf("PRESS ENTER\n");
  fgets(buf,10,stdin);
  struct filestat_list *head = procstat_getfiles(proc_stat, proc_info, 0);
 struct  filestat *fst;
  STAILQ_FOREACH(fst, head, next) {

    if (fst->fs_uflags & PS_FST_UFLAG_CDIR) {
      strcpy(cwd, fst->fs_path);
              puts(fst->fs_path);
    }
  }
  procstat_freefiles(proc_stat, head);
  procstat_freeprocs(proc_stat, proc_info);
  procstat_close(proc_stat);
  return 0;
  }
this is the test file
Code:
#!/bin/sh
echo PID is $$ - press enter to end
read a
so compile the c file
cc x.c -o x -lprocstat
start the shell script in another terminal

$ ./z.sh
PID is 12243 - press enter to end
run ./x 12243
press enter in the shell term
press enter in the ./x term
x segvs
if you dont end the shell script there is no problem
 

Jose

Daemon

Reaction score: 857
Messages: 1,033

I have it loop back to cntp - 1 when I iterate below zero, and I have it loop back to zero when it is equal to cntp going the other way. Every time I scroll the mouse wheel to change iteration the process id list I have set up to get updated automatically...
I see no such loop in your code. Don't know what the heck the mouse wheel has to do with anything.
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

I see no such loop in your code. Don't know what the heck the mouse wheel has to do with anything.
I didn't provide all the code I am using. A lot of the code that I believe is unrelated to the problem I left out. The mouse wheel was part of the steps to reproduce, but I included the code that is called when the mouse wheel is scrolled, not the actual mouse wheel handling because I thought that wouldn't be relevent.

As I mentioned in the OP, to provide all the code related to this project would have so many dependencies to install it would be unrealistic to expect anyone to want to help that much in depth, so if no one has any other ideas why it is segfauling like I mentioned in the OP I guess I can call it quits here and further attempt to debug this one on my own forgetting about getting any help.
 

Jose

Daemon

Reaction score: 857
Messages: 1,033

I didn't provide all the code I am using...
C:
#elif defined(__FreeBSD__)
  char cwd[PATH_MAX]; unsigned cntp;
  procstat *proc_stat = procstat_open_sysctl();
  kinfo_proc *proc_info = procstat_getprocs(proc_stat, KERN_PROC_PID, procId, &cntp);
  filestat_list *head = procstat_getfiles(proc_stat, proc_info, 0);
  filestat *fst;
  STAILQ_FOREACH(fst, head, next) {
    if (fst->fs_uflags & PS_FST_UFLAG_CDIR) {
      strcpy(cwd, fst->fs_path);
      static std::string str; str = cwd;
      *buffer = (char *)str.c_str();
    }
  }
  procstat_freefiles(proc_stat, head);
  procstat_freeprocs(proc_stat, proc_info);
  procstat_close(proc_stat);
 #endif
There's no loop involving cntp in that code. You don't check the value of cntp at all. The call to procstat_getprocs is probably returning 0 results, and thus proc_info is NULL. You pass it to procstat_getfiles anyway, and it very reasonably explodes in your face.
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

C:
#elif defined(__FreeBSD__)
  char cwd[PATH_MAX]; unsigned cntp;
  procstat *proc_stat = procstat_open_sysctl();
  kinfo_proc *proc_info = procstat_getprocs(proc_stat, KERN_PROC_PID, procId, &cntp);
  filestat_list *head = procstat_getfiles(proc_stat, proc_info, 0);
  filestat *fst;
  STAILQ_FOREACH(fst, head, next) {
    if (fst->fs_uflags & PS_FST_UFLAG_CDIR) {
      strcpy(cwd, fst->fs_path);
      static std::string str; str = cwd;
      *buffer = (char *)str.c_str();
    }
  }
  procstat_freefiles(proc_stat, head);
  procstat_freeprocs(proc_stat, proc_info);
  procstat_close(proc_stat);
#endif
There's no loop involving cntp in that code. You don't check the value of cntp at all. The call to procstat_getprocs is probably returning 0 results, and thus proc_info is NULL. You pass it to procstat_getfiles anyway, and it very reasonably explodes in your face.
View: https://www.youtube.com/watch?v=r-sWP7RUfW0

Explained the best I can and didn't half-ass the video this time. Hope I'm not being difficult.

if procid dies somewhere between the lprocstat calls it segvs
this is a simplified version of the cpp file
Code:
#include <sys/socket.h>
#include <sys/sysctl.h>
#include <sys/param.h>
#include <sys/queue.h>
#include <sys/user.h>
#include <libprocstat.h>
#include <libutil.h>
#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <unistd.h>

int main(int argc, char ** argv) {
char cwd[PATH_MAX]; unsigned cntp;
char buf[11];
int procId = atoi(argv[1]);
  printf("%d \n",procId);
  fflush(stdout);
  struct procstat *proc_stat = procstat_open_sysctl();
  printf("PS:%p\n",proc_stat);
  struct kinfo_proc *proc_info = procstat_getprocs(proc_stat, KERN_PROC_PID, procId, &cntp);
  printf("PI:%p\n",proc_info);

  if(!proc_info) exit(1);
  printf("PRESS ENTER\n");
  fgets(buf,10,stdin);
  struct filestat_list *head = procstat_getfiles(proc_stat, proc_info, 0);
struct  filestat *fst;
  STAILQ_FOREACH(fst, head, next) {

    if (fst->fs_uflags & PS_FST_UFLAG_CDIR) {
      strcpy(cwd, fst->fs_path);
              puts(fst->fs_path);
    }
  }
  procstat_freefiles(proc_stat, head);
  procstat_freeprocs(proc_stat, proc_info);
  procstat_close(proc_stat);
  return 0;
  }
this is the test file
Code:
#!/bin/sh
echo PID is $$ - press enter to end
read a
so compile the c file
cc x.c -o x -lprocstat
start the shell script in another terminal

$ ./z.sh
PID is 12243 - press enter to end
run ./x 12243
press enter in the shell term
press enter in the ./x term
x segvs
if you dont end the shell script there is no problem

Thank you for your response!

I am heading to bed soon but I'll be happy to check out your response more after some sleep. Thank you for the help! :)
 

mark_j

Daemon

Reaction score: 579
Messages: 1,058

C:
#elif defined(__FreeBSD__)
  char cwd[PATH_MAX]; unsigned cntp;
  procstat *proc_stat = procstat_open_sysctl();
  kinfo_proc *proc_info = procstat_getprocs(proc_stat, KERN_PROC_PID, procId, &cntp);
  filestat_list *head = procstat_getfiles(proc_stat, proc_info, 0);
  filestat *fst;
  STAILQ_FOREACH(fst, head, next) {
    if (fst->fs_uflags & PS_FST_UFLAG_CDIR) {
      strcpy(cwd, fst->fs_path);
      static std::string str; str = cwd;
      *buffer = (char *)str.c_str();
    }
  }
  procstat_freefiles(proc_stat, head);
  procstat_freeprocs(proc_stat, proc_info);
  procstat_close(proc_stat);
#endif
There's no loop involving cntp in that code. You don't check the value of cntp at all. The call to procstat_getprocs is probably returning 0 results, and thus proc_info is NULL. You pass it to procstat_getfiles anyway, and it very reasonably explodes in your face.
I guess that comes from assuming procstat_getprocs(3) always works and not testing if proc_info is not NULL?
Actually, from the code I see, most is assumed to just work. Talk about setting up a minefield and waiting for a rabbit...
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

Thanks guys. I have a lot of work on my hands it appears. lol. I guess it's fair to say I'm not reading the docs enough and that's why I don't even think to do these checks. I'll keep that in mind going forward. Would any of you like to be added the copyright as my way of saying thank you for bringing to my attention such a revelation?

Edit:

I'm still a bit interested in knowing when those functions would exactly fail besides when the process id fed to them doesn't exist. I've decided to stay up a bit longer to look into this further, the man page for these functions doesn't appear to even say any conditions which would cause these functions would fail, so we're going into undocumented behavior at this point:


There are functions that are mentioned there which throw error messages, but they are not functions I happen to be using. For educational purposes, it would be nice to know more causes there could be for these functions to fail besides lack of an existing process id.

This makes me want to play around with covacat's example code a bit.
 

mark_j

Daemon

Reaction score: 579
Messages: 1,058

Thanks guys. I have a lot of work on my hands it appears. lol. I guess it's fair to say I'm not reading the docs enough and that's why I don't even think to do these checks. I'll keep that in mind going forward. Would any of you like to be added the copyright as my way of saying thank you for bringing to my attention such a revelation?

Well the man page is a bit of a joke. Man pages are supposed to be terse (it appears) but this one is downright stingy on information.
Nothing about return codes, etc. For example, does it set errno? You couldn't tell from the man page.

However, assuming something works is always a dangerous approach. It's better to get it now than never at all.

For example, doing a search in the FreeBSD source code to see when procstat_getfiles is called would give you an indication of the tests. That's time consuming and unwarranted. Generally though, testing a function that returns a pointer for a NULL value is always a good thing[tm]. Always.

Most library functions return a value, as you know, and being lazy (and I am guilty of it too) by not checking return codes, return structures etc is always a problem.


Edit:

I'm still a bit interested in knowing when those functions would exactly fail besides when the process id fed to them doesn't exist. I've decided to stay up a bit longer to look into this further, the man page for these functions doesn't appear to even say any conditions which would cause these functions would fail, so we're going into undocumented behavior at this point:


There are functions that are mentioned there which throw error messages, but they are not functions I happen to be using. For educational purposes, it would be nice to know more causes there could be for these functions to fail besides lack of an existing process id.
What in particular are you referring to?
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

What in particular are you referring to?

Code:
int
     procstat_get_pipe_info(struct procstat *procstat, struct filestat *fst,
     struct    pipestat *pipe,    char *errbuf);

int
     procstat_get_pts_info(struct procstat *procstat, struct filestat *fst,
     struct    ptsstat    *pts, char *errbuf);

int
     procstat_get_socket_info(struct procstat *procstat, struct    filestat *fst,
     struct    sockstat *sock,    char *errbuf);

int
     procstat_get_vnode_info(struct procstat *procstat,    struct filestat    *fst,
     struct    vnstat *vn, char *errbuf);

These functions in particular have an error buffer, but I'm not using them.

I'm starting right now adding some null checks, and will report back whether that fixes the issue, and will post my updated code.4

Edit:

You guys were right! Those null checks made all the difference! Problem solved and no crash. Thank you so much for everything!

Here's the commit which fixes it:

Thanks again!
Samuel
 

mark_j

Daemon

Reaction score: 579
Messages: 1,058

Right. So we're back to the poorly documented man page.

As I said, though, testing that all the structure pointers you pass to these functions have not been set to NULL is a good start.

Checking errbuf for a message seems a lost cause though because maybe it only puts something there in error (logical but can you assume this?) or maybe it just returns "OK!" ... :) Most library functions are 0 = good, non-0 = bad.

In this case, yes, it would be great to know the return codes for these functions.
 

mark_j

Daemon

Reaction score: 579
Messages: 1,058

I just glanced through the source code of libprocstat. If there's an error in the above functions, the best you will get is errbuf set to "error". (Yes, I know, these developers are just so verbose when describing an error!)

As I suspected, they also return 0 for good, 1 for bad.

However, some (not the ones you listed above) return -1 for error, 0 for good. Some other functions return 1 for good. It's all over the place.
Likewise, some functions declare the errbuf unused, but in their defence, these are somewhat "private" functions, not designed to be accessed by a user.
I might see if I can get this man page broadened out a bit.
 
OP
Samuel Venable

Samuel Venable

Active Member

Reaction score: 105
Messages: 149

I just glanced through the source code of libprocstat. If there's an error in the above functions, the best you will get is errbuf set to "error". (Yes, I know, these developers are just so verbose when describing an error!)

As I suspected, they also return 0 for good, 1 for bad.

However, some (not the ones you listed above) return -1 for error, 0 for good. Some other functions return 1 for good. It's all over the place.
Likewise, some functions declare the errbuf unused, but in their defence, these are somewhat "private" functions, not designed to be accessed by a user.
I might see if I can get this man page broadened out a bit.
If they would accept me as a contributor, I personally would love to make better error messages and return codes that are more consistent. It's a shame I signed up a long time ago and they still haven't approved me. Probably because they saw me arguing and saying dumb things on GitHub, lol I joke around in a lot of weird ways in a particular community and if they saw how I acted over there I can see why they'd be hesitant.
 

mark_j

Daemon

Reaction score: 579
Messages: 1,058

I don't know your circumstances, but this is your opportunity. Makes changes to this lib, offer up those changes and see what happens.
 
Top