On Mon, Oct 23 2017, Jeremie Courreges-Anglas <[email protected]> wrote:
> On Mon, Oct 23 2017, "Todd C. Miller" <[email protected]> wrote:
>> On Mon, 23 Oct 2017 09:05:48 +0200, Jeremie Courreges-Anglas wrote:
>>
>>> That doesn't work. When here, we hold cronSock and dfd (should be 3 and
>>> 4). We need dfd to get fd (should be 5), thus we can't just use
>>> "closefrom(3)". The straightest way IMO is to close what we should
>>> close (including dfd), but this means making cronSock a global.
>>
>> OK millert@
>>
>>> I would also propose sprinkling more O_CLOEXEC magic but that can be in
>>> another diff.
>>
>> Sure.
>
> Here it is. Liberal use of O_CLOEXEC except for poke_daemon() which is
> more obvious (but I can add it there too if you prefer). cronSock
> already uses SOCK_CLOEXEC.
>
> ok?
Note: there may be others places where we could use close-on-exec mode
"e" for fopen, not sure about this yet. Feel free to beat me to it. :)
>
> Index: atrun.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 atrun.c
> --- atrun.c 23 Oct 2017 15:15:22 -0000 1.47
> +++ atrun.c 23 Oct 2017 15:26:45 -0000
> @@ -83,7 +83,8 @@ scan_atjobs(at_db **db, struct timespec
> struct dirent *file;
> struct stat sb;
>
> - if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
> + dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
> + if (dfd == -1) {
> syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
> return (0);
> }
> @@ -175,7 +176,8 @@ atrun(at_db *db, double batch_maxload, t
> if (db == NULL)
> return;
>
> - if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
> + dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
> + if (dfd == -1) {
> syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
> return;
> }
> @@ -262,7 +264,8 @@ run_job(const atjob *job, int dfd, const
> char *nargv[2], *nenvp[1];
>
> /* Open the file and unlink it so we don't try running it again. */
> - if ((fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < 0) {
> + fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC, 0);
> + if (fd == -1) {
> syslog(LOG_ERR, "(CRON) CAN'T OPEN (%s)", atfile);
> return;
> }
> Index: database.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/cron/database.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 database.c
> --- database.c 7 Jun 2017 23:36:43 -0000 1.35
> +++ database.c 23 Oct 2017 15:26:45 -0000
> @@ -182,7 +182,8 @@ process_crontab(int dfd, const char *una
> goto next_crontab;
> }
>
> - crontab_fd = openat(dfd, fname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW);
> + crontab_fd = openat(dfd, fname,
> + O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
> if (crontab_fd < 0) {
> /* crontab not accessible?
> */
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
