Re: [Linux-ima-user] [systemd-devel] [PATCH 2/2] main: added support for loading IMA custom policies

[Lennart P. <le...@po...>]

On Wed, 22.02.12 15:52, Roberto Sassu (rob...@po...) wrote:

Heya,

> +       policy = mmap(NULL, policy_size, PROT_READ, MAP_PRIVATE, policyfd, 0);
> +       if (policy == MAP_FAILED) {
> +               log_error("mmap() failed (%m), freezing");
> +               result = -errno;
> +               goto out;
> +       }
> +
> +       while(written < policy_size) {
> +               ssize_t len = write(imafd, policy + written,
> +                                   policy_size - written);
> +               if (len <= 0) {
> +                         if (errno == EINVAL)
> +                                   log_error("Invalid line #%d in the IMA custom policy file %s",
> +                                             policy_line_number, IMA_POLICY_PATH);
> +
> +                         log_error("Failed to load the IMA custom policy "
> +                                   "file %s (%m), ignoring.", IMA_POLICY_PATH);
> +                         goto out_mmap;
> +               }
> +               written += len;
> +               policy_line_number++;

I don't understand the counting here of policy_line_number? You attempt
to write the whole policy at once, no? How does this counting of line
numbers work here then? Or does the write() call on the kernel file
actually only accept one line at a time? If that's the case is it really
a good idea to rely on that behaviour? Knowing how these things go
eventually things might get optimized to read more than one line at once
and then the counting here will be off. Maybe it makes sense to drop the
counting entirely here?

(Something else thing that gets me thinking: by mmap()ing the source
file you imply that the policy can never grow beyond 2G or so. I presume
that's not a problem, right?)

Otherwise looks good.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.