cmd/snap-confine: nvidia: preserve globbed file prefix #4902
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4902 +/- ##
=======================================
Coverage 78.96% 78.96%
=======================================
Files 478 478
Lines 34902 34902
=======================================
Hits 27559 27559
Misses 5152 5152
Partials 2191 2191Continue to review full report at Codecov.
|
| const char *pathname = glob_res.gl_pathv[i]; | ||
| char *pathname_copy | ||
| SC_CLEANUP(sc_cleanup_string) = strdup(pathname); | ||
| char *dirname_copy | ||
| SC_CLEANUP(sc_cleanup_string) = strdup(pathname); |
There was a problem hiding this comment.
Please check for both of those being NULL.
| char *filename = basename(pathname_copy); | ||
| char *directory_name = dirname(dirname_copy); | ||
| strncpy(prefix_dir, libgl_dir, sizeof prefix_dir); |
There was a problem hiding this comment.
Can you please use sc_must_snprintf instead, it's a bit easier to get right than knowing exactly how strncpy fails.
| char *directory_name = dirname(dirname_copy); | ||
| strncpy(prefix_dir, libgl_dir, sizeof prefix_dir); | ||
|
|
||
| if (strlen(directory_name) > source_dir_len) { |
There was a problem hiding this comment.
Hmm, can you add a comment what is going on here? I'm a bit slow today.
| @@ -123,6 +125,7 @@ static void sc_populate_libgl_with_hostfs_symlinks(const char *libgl_dir, | |||
| const char *glob_list[], | |||
| size_t glob_list_len) | |||
| { | |||
| size_t source_dir_len = strlen(source_dir); | |||
There was a problem hiding this comment.
strlen() can be tricky with untrusted inputs, but I verified the callers of this function and all of the inputs are trusted.
|
I'll amend & force push if you don't mind. This will make cherry-picking easier in case we need it. |
bboozzoo
force-pushed
the
bboozzoo/nvidia-preserve-glob-prefix
branch
2 times, most recently
from
430da86
to
9c61fb7
Compare
| const char *pathname = glob_res.gl_pathv[i]; | ||
| char *pathname_copy | ||
| SC_CLEANUP(sc_cleanup_string) = strdup(pathname); | ||
| char *dirname_copy | ||
| SC_CLEANUP(sc_cleanup_string) = strdup(pathname); |
There was a problem hiding this comment.
Did you mean to strdup pathname again? It looks like a bug
There was a problem hiding this comment.
Yes I totally mean it. POSIX dirname() modifies its input argument.
There was a problem hiding this comment.
Can you call those pathname_copy1 and pathname_copy2 and add a comment describing why we need them. A naive quick look (I agree this is right but just confusing) may mistake this for copy-paste error.
When symlinking libgl files from hostfs we discard the path prefix inside
source_dir of a globbed file and overwrite already symlinked files if those are
created before.
For example, nvidia globs:
libnvidia-tls.so*
tls/libnvidia-tls.so*
Produce the following matches:
${source_dir}/libnvidia-tls.so
${source_dir}/libnvidia-tls.so.390.42
${source_dir}/tls/libnvidia-tls.so
${source_dir}/tls/libnvidia-tls.so.390.42
When the files under ${source_dir}/tls are processed, eg.
${source_dir}/tls/libnvidia-tls.so.390.42 the symlink we create will
overwrite ${libgl_dir}/libnvidia-tls.so.390.42, thus shadowing the original
library that was present at this path.
The patch attempts to fix this behavior and mirrors the source_dir globs, thus
the following symlinks are created:
${libgl_dir}/libnvidia-tls.so.390.42 -> ${source_dir}/libnvidia-tls.so.390.42
${libgl_dir}/tls/libnvidia-tls.so.390.42 -> ${source_dir}/tsls/libnvidia-tls.so.390.42
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
bboozzoo
force-pushed
the
bboozzoo/nvidia-preserve-glob-prefix
branch
from
9c61fb7
to
924719e
Compare
When symlinking libgl files from hostfs we discard the path prefix inside
source_dir of a globbed file and overwrite already symlinked files if those are
created before.
For example, nvidia globs:
libnvidia-tls.so*tls/libnvidia-tls.so*Produce the following matches:
${source_dir}/libnvidia-tls.so${source_dir}/libnvidia-tls.so.390.42${source_dir}/tls/libnvidia-tls.so${source_dir}/tls/libnvidia-tls.so.390.42When the files under
${source_dir}/tlsare processed, eg.${source_dir}/tls/libnvidia-tls.so.390.42, the symlink we create willoverwrite
${libgl_dir}/libnvidia-tls.so.390.42, thus shadowing the originallibrary that was present at this path.
The patch attempts to fix this behavior and mirrors the
source_dirglobs, thusthe following symlinks are created:
${libgl_dir}/libnvidia-tls.so.390.42 -> ${source_dir}/libnvidia-tls.so.390.42${libgl_dir}/tls/libnvidia-tls.so.390.42 -> ${source_dir}/tsls/libnvidia-tls.so.390.42