[libvirt] [PATCHv2 2/8] audit: split cgroup audit types to allow more information

[Daniel P. Berrange]

On Tue, Mar 08, 2011 at 10:13:44PM -0700, Eric Blake wrote:
> Device names can be manipulated, so it is better to also log
> the major/minor device number corresponding to the cgroup ACL
> changes that libvirt made.  This required some refactoring
> of the relatively new qemu cgroup audit code.
> 
> Also, qemuSetupChardevCgroup was only auditing on failure, not success.


> +/* Return rdev=nn:mm in hex for block and character devices, rdev=?
> + * for other file types or stat failure, or NULL on allocation
> + * failure.  */
> +#if defined major && defined minor
> +static char *
> +qemuAuditGetRdev(const char *path)
> +{
> +    char *ret;
> +    struct stat sb;
> +
> +    if (stat(path, &sb) == 0 &&
> +        (S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode))) {
> +        int maj = major(sb.st_rdev);
> +        int min = minor(sb.st_rdev);
> +        virAsprintf(&ret, "rdev=%02X:%02X", maj, min);
> +    } else {
> +        ret = strdup("rdev=?");
> +    }
> +    return ret;
> +}
> +#else
> +static char *
> +qemuAuditGetRdev(const char *path ATTRIBUTE_UNUSED)
> +{
> +    return strdup("rdev=?");
> +}
> +#endif

Rather than have the two  strdup("rdev=?")  calls, I reckon it
would be better to just return NULL. Then the caller can just
check for NULL itself & fallback to a static  "rdev=?".

In fact, perhaps this should just do

       virAsprintf(&ret, "%02X:%02X", maj, min);

And...

> +void
> +qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup,
> +                    const char *reason, const char *path, int rc)
> +{
> +    char *detail;
> +    char *rdev;
> +    char *extra;
> +
> +    /* Nothing to audit for regular files.  */
> +    if (rc > 0)
> +        return;
> +
> +    if (!(detail = virAuditEncode("path", path)) ||
> +        !(rdev = qemuAuditGetRdev(path)) ||
> +        virAsprintf(&extra, "path path=%s %s", path, rdev) < 0) {

...here do 

        virAsprintf(&extra, "path path=%s rdev=%s", path, VIR_AUDIT_STR(rdev)) < 0) {



ACK, to the rest of the patch though.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|