Report error to user if image activator fails with ENOMEM

[emaste]

Summary

Previously kern_exec silently failed, killing the process with SIGABRT.

Posted for discussion. My take is that ENOMEM is a useful error to report, but others may not have much value.

[kib]

I would not object against addition of the uprintf(), possibly managed to silence with some sysctl. Look at the kern_exec.c disallow_high_osrel which might be covered by the same knob.

But I think that to be useful, we should add uprintf() to image activators error situations, and not to the summary error path in the kern_exec.c. I.e. I suggest to annotate with uprintf() imgact_elf.c. You would be able to provide much more precise and up to the point diagnostic there. Like, in your case, "text segment too large".

[emaste]

In D2731#51705, @kib wrote:

But I think that to be useful, we should add uprintf() to image activators error situations

Ok, I agree that will provide a much better user-facing error, and will post a new patch shortly.

[emaste]

Move reporting into imgact_elf.

[kib]

This could be made much shorter if you create a local variable of type char *, initialized to NULL. In the 'then' blocks of if()s, the var is asssigned a message, nothing more. Then, the final if(var != NULL) {PROC_UNLOCK(); uprintf();return();} is added.

Also, there are several other places in the imgact_elf.c where similar messages could be added in load_file(), e.g. e_type check, check for headers living in the first page, alignment etc.

[emaste]

Use error message string pointer as suggested by kib, and add reporting for the other failure cases

[emaste]

sys/kern/imgact_elf.c

912 ↗

(On Diff #5963)

I could also combine RLIMIT_DATA and RACCT_DATA into a single "Data segment size exceeds limit" and similar for _VMEM. Having distinct messages allows for finer-grained troubleshooting steps, but perhaps it's not necessary.

[kib]

sys/kern/imgact_elf.c

758 ↗

(On Diff #5963)

It is program headerS, at least I saw this usage most of the time, I believe.

780 ↗

(On Diff #5963)

This is not correct, I think. This is not about interpreter, but about interpreter header, pointing to the file name to use. Might be, 'Invalid PT_INTERP' is enough ?

804 ↗

(On Diff #5963)

'Cannot execute shared object'

[emaste]

Improve error messages based on kib's feedback

[kib]

sys/kern/imgact_elf.c

735 ↗

(On Diff #6004)

Could you do a follow-up, after this patch is committed, to clear the declaration blocks in the imgact_elf.c ? Initializations should go into the code blocks; vars should be reordered, pointers first, primitive arithmetic types afterward.