Differential D1009 Diff 2114 usr.bin/elfdump/elfdump.c

Changeset View

Standalone View

usr.bin/elfdump/elfdump.c

Context not available.
	#include <sys/cdefs.h>	#include <sys/cdefs.h>
	__FBSDID("$FreeBSD$");	__FBSDID("$FreeBSD$");

		#include <sys/capsicum.h>
	#include <sys/types.h>	#include <sys/types.h>
		pjdUnsubmitted Not Done Inline Actions You should always include sys/types.h first. It is good to sort kernel headers, but it is not always possible (it should always be possible for userland headers, though). sys/types.h and sys/param.h are exceptions and should always be included first (but only one of them). pjd: You should always include sys/types.h first. It is good to sort kernel headers, but it is not…
		jonathanUnsubmitted Not Done Inline Actions This is an extremely picky point, but should we keep the newline between sys/types.h and the other sys/ headers? That is: #include <sys/types.h> #include <sys/capsicum.h> #include <sys/elf32.h> /* ... / jonathan:* This is an //extremely// picky point, but should we keep the newline between sys/types.h and…
	#include <sys/elf32.h>	#include <sys/elf32.h>
	#include <sys/elf64.h>	#include <sys/elf64.h>
Context not available.
	#include <sys/mman.h>	#include <sys/mman.h>
	#include <sys/stat.h>	#include <sys/stat.h>
	#include <err.h>	#include <err.h>
		#include <errno.h>
	#include <fcntl.h>	#include <fcntl.h>
	#include <inttypes.h>	#include <inttypes.h>
	#include <stddef.h>	#include <stddef.h>
Context not available.
	int fd;	int fd;
	int ch;	int ch;
	int i;	int i;
		cap_rights_t rights;
		jonathanUnsubmitted Not Done Inline Actions Should `cap_rights_t rights` be declared earlier in `main`? jonathan: Should `cap_rights_t rights` be declared earlier in `main`?
		bruefferAuthorUnsubmitted Not Done Inline Actions I looked for a style(7) rule for this, but couldn't find one. Any advice appreciated :-) brueffer: I looked for a style(7) rule for this, but couldn't find one. Any advice appreciated :-)
		jonathanUnsubmitted Not Done Inline Actions My understanding is that we sort in descending order of sizeof(), then ascending alphabetical order. I also couldn't find a hard-and-fast rule in style(9), but I think this is the generally-accepted principle. Given my relationship with style(9), however, you should take this comment with a rather large grain of salt. jonathan: My understanding is that we sort in descending order of sizeof(), then ascending alphabetical…

	out = stdout;	out = stdout;
	flags = 0;	flags = 0;
Context not available.
	case 'w':	case 'w':
	if ((out = fopen(optarg, "w")) == NULL)	if ((out = fopen(optarg, "w")) == NULL)
	err(1, "%s", optarg);	err(1, "%s", optarg);
		cap_rights_init(&rights, CAP_WRITE);
		if (cap_rights_limit(fileno(out), &rights) < 0 && errno != ENOSYS)
		err(1, "unable to limit rights for %s", optarg);
	break;	break;
	case '?':	case '?':
	default:	default:
Context not available.
	if ((fd = open(*av, O_RDONLY)) < 0 \|\|	if ((fd = open(*av, O_RDONLY)) < 0 \|\|
	fstat(fd, &sb) < 0)	fstat(fd, &sb) < 0)
	err(1, "%s", *av);	err(1, "%s", *av);

		cap_rights_init(&rights, CAP_MMAP_R);
		if (cap_rights_limit(fd, &rights) < 0 && errno != ENOSYS)
		err(1, "unable to limit rights for %s", *av);
		jonathanUnsubmitted Not Done Inline Actions Should we also limit `stdout` and `stderr`? jonathan: Should we also limit `stdout` and `stderr`?
		bruefferAuthorUnsubmitted Not Done Inline Actions Maybe, should this always be done? brueffer: Maybe, should this always be done?
		jonathanUnsubmitted Not Done Inline Actions It's a good idea to: you wouldn't want a malicious ELF binary to gain control of your terminal session (e.g., be able to send funny ioctls) when you inspect it. Such highjacking should be reserved for when you run a malicious binary. :) jonathan: It's a good idea to: you wouldn't want a malicious ELF binary to gain control of your terminal…

		jonathanUnsubmitted Not Done Inline Actions Maybe STDIN_FILENO as well? Or even better, maybe close(STDIN_FILENO) ? jonathan: Maybe STDIN_FILENO as well? Or even better, maybe close(STDIN_FILENO) ?
		if (cap_enter() < 0 && errno != ENOSYS)
		err(1, "unable to enter capability mode");

	e = mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);	e = mmap(NULL, sb.st_size, PROT_READ, MAP_SHARED, fd, 0);
	if (e == MAP_FAILED)	if (e == MAP_FAILED)
	err(1, NULL);	err(1, NULL);
Context not available.