/snap-confine

Towards safe mount #65

Merged
merged 3 commits into from Jun 30, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@zyga @kyrofa
@zyga
Collaborator

zyga commented Jun 30, 2016

Just more functions and more tests

zyga added some commits Jun 30, 2016

@zyga
Add is_subdir() from lxd 
This patch is a copy of a function found in
lxc/lxc@592fd47
by Serge Hallyn <serge.hallyn@ubuntu.com>.

The committer at the time was a Canonical employee. The code is
implicitly relicensed to match the license in snap-confine
a175c29
@zyga
Add tests for is_subdir

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
5ec6ffd
src/mount-support.c
+ * @len is the length of @dir (to avoid having to recalculate it).
+**/
+static bool __attribute__ ((used))
+ is_subdir(const char *subdir, const char *dir, size_t len)
@kyrofa

kyrofa Jun 30, 2016

Member

How to use this function is not obvious from its signature. You documented this (thank you), but perhaps it would be more clear to accept a length for both of these?

@kyrofa

kyrofa Jun 30, 2016

Member

Also, this doesn't take into account .. etc. I assume that's okay?

@zyga

zyga Jun 30, 2016

Collaborator

This function comes straight from upstream LXD codebase. I agree on the size being awkward to pass. The original code doesn't really take any advantage of this. I will change the method to compute both sizes by itself.

@kyrofa

kyrofa Jun 30, 2016

Member

I will change the method to compute both sizes by itself.

Yeah that's the ideal solution.

@zyga

zyga Jun 30, 2016

Collaborator

Ha, I guess this was an useful exercise. That function is buggy

@zyga
Refactor and fix is_subdir 
This patch fixes is_subdir not to look at dir[dirlen - 1] when dirlen
is zero.

I’ve also removed the third argument as there is not benefit from
having it and it a potential source of errors and confusion.

All checks have passed

2 successful checks
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
continuous-integration/travis-ci/push — The Travis CI build passed
Details
6642494
src/mount-support.c
+ // @dir can look like "path/" (that is, end with the directory separator).
+ // When that is the case then given the test above we can be sure @subdir
+ // is a real subdirectory.
+ if (dirlen > 0 && dir[dirlen - 1] == '/')
@kyrofa

kyrofa Jun 30, 2016

Member

Ah yeah, good catch.

@kyrofa
Member

kyrofa commented Jun 30, 2016

👍 looks good to me.

@kyrofa kyrofa merged commit 938c748 into master Jun 30, 2016
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@zyga zyga deleted the towards-safe-mount branch Jul 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment