[systemd-devel] [PATCH] core: reuse the same /tmp, /var/tmp and inaccessible dir

[Lennart Poettering]

On Wed, 13.02.13 14:45, Michal Sekletar (msekleta at redhat.com) wrote:

Heya!

Looks good in principle.

> +        if (context->private_tmp) {
> +                if (!context->tmp_dir) {
> +                        d = mktemp(tmp_dir);

mktemp() is never OK. Also see "BUGS" section in the man page about
that. Use mkdtemp() or so instead.

> +                        if (!d) {
> +                                r = -errno;
> +                                goto fail;
> +                        }
> +
> +                        context->tmp_dir = strdup(d);
> +                        if (!context->tmp_dir) {
> +                                r = log_oom();
> +                                goto fail;
> +                        }
> +
> +                        u = umask(0000);
> +                        r = mkdir(tmp_dir, 0777);
> +                        umask(u);
> +                        if (r < 0) {
> +                                free(context->tmp_dir);
> +                                context->tmp_dir = NULL;
> +                                r = -errno;
> +                                goto fail;
> +                        }
> +                        remove_tmp = true;
> +
> +                        if (chmod(tmp_dir, 0777 | S_ISVTX) < 0) {
> +                                r = -errno;
> +                                goto fail;
> +                        }
> +                }
>  
> -                p->path = "/var/tmp";
> -                p->mode = PRIVATE_VAR_TMP;
> -                p++;
> +                if (!context->var_tmp_dir) {
> +                        d = mktemp(var_tmp_dir);
> +                        if (!d) {
> +                                r = -errno;
> +                                goto fail;
> +                        }
> +
> +                        context->var_tmp_dir = strdup(d);
> +                        if (!context->var_tmp_dir) {
> +                                r = log_oom();
> +                                goto fail;
> +                        }
> +
> +                        u = umask(0000);
> +                        r = mkdir(var_tmp_dir, 0777);
> +                        umask(u);
> +                        if (r < 0) {
> +                                free(context->var_tmp_dir);
> +                                context->var_tmp_dir = NULL;
> +                                r = -errno;
> +                                goto fail;
> +                        }
> +                        remove_var_tmp = true;
> +
> +                        if (chmod(var_tmp_dir, 0777 | S_ISVTX) < 0) {
> +                                r = -errno;
> +                                goto fail;
> +                        }


These two blocks really look like they want to be replaced by a function
that we just can call twice...

>  static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) {
>          int r;
>          assert(s);
> @@ -2520,6 +2526,10 @@ static int service_stop(Unit *u) {
>                 s->state == SERVICE_EXITED);
>  
>          service_enter_stop(s, SERVICE_SUCCESS);
> +
> +        /* we want empty tmp dirs when service is started again */
> +        service_cleanup_tmpdirs(s);
> +

This should probably move to service_enter_dead() or so.

Also, we should have the same in socket.c, mount.c, swap.c probably, as
they also use exec context...

Lennart

-- 
Lennart Poettering - Red Hat, Inc.