Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault on MacBook M1 due to unlimited file descriptors #63

Closed
badboy opened this issue Feb 9, 2021 · 9 comments
Closed

Comments

@badboy
Copy link

badboy commented Feb 9, 2021

I get segmentation faults with the latest version of entr (4.7 or git checkout).
This happens on a MacBook M1 (Arm chip), running macOS Big Sur, not much changed from the default configuration.

Backtrace:

(lldb) process launch -i input.txt  -- -s 'make'
Process 36108 launched: '/Users/jer/code/entr/entr' (arm64)
cur: 9223372036854775807
max: 9223372036854775807
Process 36108 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x00000001000023d8 entr`process_input(file=0x00000001f9273240, files=0x0000000000000000, max_files=9223372036854775807) at entr.c:305:19
   302 				continue;
   303 			}
   304 			if (S_ISREG(sb.st_mode) != 0) {
-> 305 				files[n_files] = malloc(sizeof(WatchFile));
   306 				strlcpy(files[n_files]->fn, buf, MEMBER_SIZE(WatchFile, fn));
   307 				files[n_files]->is_dir = 0;
   308 				files[n_files]->file_count = 0;

I added some logging:

n_files here is 0. max_files is -1.

After getrlimit(RLIMIT_NOFILE) I get:

rl.rlim_cur = 9223372036854775807
rl.rlim_max = 9223372036854775807

that.
That's UINT64_MAX That's INT64_MAX. sysconf(_SC_OPEN_MAX) also returns that.

As rlim_cur is the minimum out of all of that it's INT64_MAX before being passed to process_input. Now process_input takes an int, so it's coerced and becomes -1.

Seems my file descriptors are unlimited by default:

$ ulimit -n
unlimited

Changing it to some more reasonable low number makes entr work as expected:

$ ulimit -n 1024
$

Question:

  • Should entr enforce a more reasonable limit for this case?
  • Should process_file maybe not use a different integer type than the one passed in?
@eradman
Copy link
Owner

eradman commented Feb 9, 2021

Thanks for reporting this; I'm surprised because my older MacBook Air running 11.1 reports more reasonable limits

rlim_cur: 65536
rlim_max: 524288
_SC_OPEN_MAX: 65536

#include <stdio.h>
#include <unistd.h>
#include <sys/resource.h>

int main() {
        struct rlimit rl;
        getrlimit(RLIMIT_NOFILE, &rl);

        printf("rlim_cur: %lu\n", (unsigned long)rl.rlim_cur);
        printf("rlim_max: %lu\n", (unsigned long)rl.rlim_max);
        printf("_SC_OPEN_MAX: %lu\n", (unsigned long)sysconf(_SC_OPEN_MAX));
}

@badboy
Copy link
Author

badboy commented Feb 9, 2021

Yeah, I'm not sure why I have this unlimited by default. Of course I can configure it to make it work.

The actual problem in the code is this line:

files = calloc(rl.rlim_cur+1, sizeof(WatchFile *));

rl.rlim_cur+1 will be huge, multipled by sizeof(Watchfile *) will overflow, therefore calloc will fail and return a null pointer.
Question is if you want to add some code to either introduce an arbitrary limit in this case or at least error out if its clear that it is "unlimited" (which really just means "huge")?

@eradman
Copy link
Owner

eradman commented Feb 10, 2021

@badboy: please build and run this test program and tell me what the output is? I wonder if errorno is set

#include <stdio.h>
#include <unistd.h>
#include <err.h>

int main() {
        long open_max;
        open_max = sysconf(_SC_OPEN_MAX);

        if (open_max == -1)
                err(1, "_SC_OPEN_MAX");
        else
                printf("_SC_OPEN_MAX: %lu\n", (unsigned long)open_max);
}

@badboy
Copy link
Author

badboy commented Feb 10, 2021

[~]
❯ clang -o openmax openmax.c

[~]
❯ ./openmax
_SC_OPEN_MAX: 9223372036854775807

Nope, sysconf works fine.


From some peopel I asked it seems their M1 machines all have ulimit -n set to 256 as one would expect (and as does my Intel mac). So I'm not sure why my machine is configured differently. Of course this doesn't change the fact that one can configure it that way and it triggers the segfault.

@eradman
Copy link
Owner

eradman commented Feb 10, 2021

Fascinating... it really is returning 2**63 - 1

Agreed; regardless of why this value is set, entr needs to detect this condition. Maybe this?

 if (open_max > 2**31)
   open_max = 65536;

@badboy
Copy link
Author

badboy commented Feb 10, 2021

Seems reasonable to me.

@eradman
Copy link
Owner

eradman commented Feb 10, 2021

I pushed commit 8f224fb to address this. I picked a hard limit of 524288 somewhat arbitrarily, since this is the normal hard limit I see on MacOS 11.1

@badboy
Copy link
Author

badboy commented Feb 10, 2021

👍 works for me.

@badboy badboy closed this as completed Feb 10, 2021
@eradman
Copy link
Owner

eradman commented Feb 26, 2021

This fix was included in the 4.8 release today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants