[libvirt] [PATCH] storage: Sanitize pool target paths
Cole Robinson
crobinso at redhat.com
Fri May 21 17:05:50 UTC 2010
On 05/20/2010 03:28 PM, Eric Blake wrote:
> On 05/20/2010 10:04 AM, Cole Robinson wrote:
>> Spurious / in a pool target path makes life difficult for apps using the
>> GetVolByPath, and doing other path based comparisons with pools. This
>> has caused a few issues for virt-manager users:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=494005
>> https://bugzilla.redhat.com/show_bug.cgi?id=593565
>>
>> Add a new util API which removes spurious /, virFileSanitizePath. Sanitize
>> target paths when parsing pool XML, and for paths passed to GetVolByPath.
>
> In general, I like the idea, whether or not we should be moving the
> sanitization into a gnulib function. But I don't think it's ready for
> an ack quite yet.
>
>> index e937d39..8f86ed6 100644
>> --- a/src/util/util.c
>> +++ b/src/util/util.c
>> @@ -1921,6 +1921,41 @@ int virFileAbsPath(const char *path, char **abspath)
>> return 0;
>> }
>>
>> +/* Remove spurious / characters from a path. The result must be freed */
>> +char *
>> +virFileSanitizePath(const char *path)
>> +{
>> + const char *cur = path;
>> + char *cleanpath;
>> + int idx = 0;
>> +
>> + cleanpath = strdup(path);
>> + if (!cleanpath) {
>> + virReportOOMError();
>> + return NULL;
>> + }
>> +
>> + /* Sanitize path in place */
>> + while (*cur != '\0') {
>> + if (*cur == '/') {
>> + /* Skip all extra / */
>> + while (*++cur == '/')
>> + continue;
>
> POSIX says that you must be careful of "//file" - you cannot blindly
> simplify to "/file" (case in point - cygwin). But POSIX also says that
> "///file" sanitizes to "/file".
>
>> +
>> + /* Don't add a trailing / */
>> + if (*cur == '\0')
>> + break;
>> +
>> + cleanpath[idx++] = '/';
>> + }
>> +
>> + cleanpath[idx++] = *cur++;
>> + }
>> + cleanpath[idx] = '\0';
>> +
>> + return cleanpath;
>
> Sanitizing "a/b/../c" to "a/c" might not always be the right thing (what
> if a is a symlink?), but it would also be safe to sanitize "a/./b" to "a/b".
>
>> +}
>> +
>> /* Like strtol, but produce an "int" result, and check more carefully.
>> Return 0 upon success; return -1 to indicate failure.
>> When END_PTR is NULL, the byte after the final valid digit must be NUL.
>> diff --git a/src/util/util.h b/src/util/util.h
>> index 6bf6bcc..abc2688 100644
>> --- a/src/util/util.h
>> +++ b/src/util/util.h
>> @@ -118,6 +118,8 @@ char *virFindFileInPath(const char *file);
>>
>> int virFileExists(const char *path);
>>
>> +char *virFileSanitizePath(const char *path);
>
> Mark this with ATTRIBUTE_NONNULL(1).
>
Thanks for the review, updated patch sent.
- Cole
More information about the libvir-list
mailing list