On 04/11/2014 12:28 PM, Eric Blake wrote:
On 04/10/2014 10:02 PM, Jincheng Miao wrote:
If volume name is a path, storageVolCreateXML appends that name to
the specified pool path, that will taint other pools. Adding the
volume check is better for sanity.
Signed-off-by: Jincheng Miao <jmiao@xxxxxxxxxx>
---
src/storage/storage_driver.c | 9 +++++++++
1 file changed, 9 insertions(+)
+ /* Make sure the volume name is not a path */
+ if (last_component(newvol->name) != newvol->name) {
Why not just:
if (strchr(newvol->name, '/'))
sure.
Also, shouldn't we forbid things like "." and ".." as the newvol name?
I think we should just forbid '/' in the newvol name.
Why that is illegal? Consider this situation:
There is a directory pool named 'taint-pool' for a *normal user*.
If privileged user creates a volume clone, with path-based name, pointer
to this 'taint-pool'.
Then this normal user could see this volume clone after pool-refresh.
If normal user vol-delete this volume clone, then this clone belongs to
privileged user gone.
I think that is insane. If you agree with it, I will send V2 patch with
modification.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("storage volume name '%s' is a path"),
I don't like the word "path" in a user-facing error message for anything
other than a list of directory names separated by colon; better would be
a message such as "requested storage volume name '%s' cannot contain /"
or "requested storage volume name '%s' not permitted".
Yes, "requested storage volume name '%s' cannot contain /" is more precise.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list