Change OstreeFetcher to be dirfd-relative

This is a noticeable cleanup, and fixes another big user of GFile* in
performance/security sensitive codepaths.

I'm specifically making this change because the static deltas code was
leaking temporary files, and cleaning that up nicely would be best if
we were fd relative.
This commit is contained in:
Colin Walters 2015-01-14 22:04:06 -05:00
parent a7300a828d
commit 9020fe2547
7 changed files with 165 additions and 97 deletions

View File

@ -23,6 +23,7 @@
#include "config.h"
#include <gio/gfiledescriptorbased.h>
#include <gio/gunixoutputstream.h>
#include "ostree-fetcher.h"
#ifdef HAVE_LIBSOUP_CLIENT_CERTS
@ -50,7 +51,7 @@ typedef struct {
gboolean is_stream;
GInputStream *request_body;
GFile *out_tmpfile;
char *out_tmpfile;
GOutputStream *out_stream;
guint64 max_size;
@ -83,7 +84,6 @@ pending_uri_free (OstreeFetcherPendingURI *pending)
soup_uri_free (pending->uri);
g_clear_object (&pending->self);
g_clear_object (&pending->out_tmpfile);
g_clear_object (&pending->request);
g_clear_object (&pending->request_body);
g_clear_object (&pending->out_stream);
@ -95,7 +95,7 @@ struct OstreeFetcher
{
GObject parent_instance;
GFile *tmpdir;
int tmpdir_dfd;
GTlsCertificate *client_cert;
@ -126,7 +126,6 @@ _ostree_fetcher_finalize (GObject *object)
self = OSTREE_FETCHER (object);
g_clear_object (&self->session);
g_clear_object (&self->tmpdir);
g_clear_object (&self->client_cert);
g_hash_table_destroy (self->sending_messages);
@ -216,18 +215,24 @@ _ostree_fetcher_init (OstreeFetcher *self)
}
OstreeFetcher *
_ostree_fetcher_new (GFile *tmpdir,
_ostree_fetcher_new (int tmpdir_dfd,
OstreeFetcherConfigFlags flags)
{
OstreeFetcher *self = (OstreeFetcher*)g_object_new (OSTREE_TYPE_FETCHER, NULL);
self->tmpdir = g_object_ref (tmpdir);
self->tmpdir_dfd = tmpdir_dfd;
if ((flags & OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE) > 0)
g_object_set ((GObject*)self->session, "ssl-strict", FALSE, NULL);
return self;
}
int
_ostree_fetcher_get_dfd (OstreeFetcher *fetcher)
{
return fetcher->tmpdir_dfd;
}
void
_ostree_fetcher_set_proxy (OstreeFetcher *self,
const char *http_proxy)
@ -296,8 +301,7 @@ finish_stream (OstreeFetcherPendingURI *pending,
GError **error)
{
gboolean ret = FALSE;
goffset filesize;
gs_unref_object GFileInfo *file_info = NULL;
struct stat stbuf;
/* Close it here since we do an async fstat(), where we don't want
* to hit a bad fd.
@ -310,11 +314,11 @@ finish_stream (OstreeFetcherPendingURI *pending,
}
pending->state = OSTREE_FETCHER_STATE_COMPLETE;
file_info = g_file_query_info (pending->out_tmpfile, OSTREE_GIO_FAST_QUERYINFO,
G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
pending->cancellable, error);
if (!file_info)
goto out;
if (fstatat (pending->self->tmpdir_dfd, pending->out_tmpfile, &stbuf, AT_SYMLINK_NOFOLLOW) != 0)
{
gs_set_error_from_errno (error, errno);
goto out;
}
/* Now that we've finished downloading, continue with other queued
* requests.
@ -322,15 +326,14 @@ finish_stream (OstreeFetcherPendingURI *pending,
pending->self->outstanding--;
ostree_fetcher_process_pending_queue (pending->self);
filesize = g_file_info_get_size (file_info);
if (filesize < pending->content_length)
if (stbuf.st_size < pending->content_length)
{
g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Download incomplete");
goto out;
}
else
{
pending->self->total_downloaded += g_file_info_get_size (file_info);
pending->self->total_downloaded += stbuf.st_size;
}
ret = TRUE;
@ -489,10 +492,13 @@ on_request_sent (GObject *object,
if (!pending->is_stream)
{
pending->out_stream = G_OUTPUT_STREAM (g_file_append_to (pending->out_tmpfile, G_FILE_CREATE_NONE,
pending->cancellable, &local_error));
if (!pending->out_stream)
goto out;
int fd = openat (pending->self->tmpdir_dfd, pending->out_tmpfile, O_CREAT | O_WRONLY | O_APPEND | O_CLOEXEC, 0600);
if (fd == -1)
{
gs_set_error_from_errno (&local_error, errno);
goto out;
}
pending->out_stream = g_unix_output_stream_new (fd, TRUE);
g_hash_table_add (pending->self->output_stream_set, g_object_ref (pending->out_stream));
g_input_stream_read_bytes_async (pending->request_body, 8192, G_PRIORITY_DEFAULT,
pending->cancellable, on_stream_read, pending);
@ -527,7 +533,6 @@ ostree_fetcher_request_uri_internal (OstreeFetcher *self,
gpointer source_tag)
{
OstreeFetcherPendingURI *pending = g_new0 (OstreeFetcherPendingURI, 1);
GFile *out_tmpfile = NULL;
GError *local_error = NULL;
pending->refcount = 1;
@ -560,26 +565,38 @@ ostree_fetcher_request_uri_internal (OstreeFetcher *self,
}
else
{
gs_unref_object GFileInfo *file_info = NULL;
gs_free char *uristring = soup_uri_to_string (uri, FALSE);
gs_free char *hash = g_compute_checksum_for_string (G_CHECKSUM_SHA256, uristring, strlen (uristring));
out_tmpfile = g_file_get_child (self->tmpdir, hash);
if (!ot_gfile_query_info_allow_noent (out_tmpfile, OSTREE_GIO_FAST_QUERYINFO,
G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
&file_info, cancellable, &local_error))
goto fail;
gs_free char *tmpfile = NULL;
struct stat stbuf;
gboolean exists;
tmpfile = g_compute_checksum_for_string (G_CHECKSUM_SHA256, uristring, strlen (uristring));
if (fstatat (self->tmpdir_dfd, tmpfile, &stbuf, AT_SYMLINK_NOFOLLOW) == 0)
exists = TRUE;
else
{
if (errno == ENOENT)
exists = FALSE;
else
{
gs_set_error_from_errno (&local_error, errno);
goto fail;
}
}
if (SOUP_IS_REQUEST_HTTP (pending->request))
{
SoupMessage *msg;
msg = soup_request_http_get_message ((SoupRequestHTTP*) pending->request);
if (file_info && g_file_info_get_size (file_info) > 0)
soup_message_headers_set_range (msg->request_headers, g_file_info_get_size (file_info), -1);
if (exists && stbuf.st_size > 0)
soup_message_headers_set_range (msg->request_headers, stbuf.st_size, -1);
g_hash_table_insert (self->message_to_request,
soup_request_http_get_message ((SoupRequestHTTP*)pending->request),
pending);
}
pending->out_tmpfile = out_tmpfile;
pending->out_tmpfile = tmpfile;
tmpfile = NULL; /* Transfer ownership */
g_queue_insert_sorted (&self->pending_queue, pending, pending_uri_compare, NULL);
ostree_fetcher_process_pending_queue (self);
@ -612,7 +629,7 @@ _ostree_fetcher_request_uri_with_partial_async (OstreeFetcher *self,
_ostree_fetcher_request_uri_with_partial_async);
}
GFile *
char *
_ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
GAsyncResult *result,
GError **error)
@ -627,7 +644,7 @@ _ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
return NULL;
pending = g_simple_async_result_get_op_res_gpointer (simple);
return g_object_ref (pending->out_tmpfile);
return g_strdup (pending->out_tmpfile);
}
static void

View File

@ -54,8 +54,10 @@ typedef enum {
GType _ostree_fetcher_get_type (void) G_GNUC_CONST;
OstreeFetcher *_ostree_fetcher_new (GFile *tmpdir,
OstreeFetcherConfigFlags flags);
OstreeFetcher *_ostree_fetcher_new (int tmpdir_dfd,
OstreeFetcherConfigFlags flags);
int _ostree_fetcher_get_dfd (OstreeFetcher *fetcher);
void _ostree_fetcher_set_proxy (OstreeFetcher *fetcher,
const char *proxy);
@ -76,7 +78,7 @@ void _ostree_fetcher_request_uri_with_partial_async (OstreeFetcher *self
GAsyncReadyCallback callback,
gpointer user_data);
GFile *_ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
char *_ostree_fetcher_request_uri_with_partial_finish (OstreeFetcher *self,
GAsyncResult *result,
GError **error);

View File

@ -21,6 +21,7 @@
#include "config.h"
#include "ostree-metalink.h"
#include <gio/gfiledescriptorbased.h>
#include "otutil.h"
#include "libgsystem.h"
@ -72,7 +73,7 @@ typedef struct
char *verification_sha256;
char *verification_sha512;
GFile *result;
char *result;
char *last_metalink_error;
guint current_url_index;
@ -429,34 +430,46 @@ on_fetched_url (GObject *src,
GTask *task = user_data;
OstreeMetalinkRequest *self = g_task_get_task_data (task);
GError *local_error = NULL;
gs_unref_object GFile *result = NULL;
gs_unref_object GFileInfo *finfo = NULL;
struct stat stbuf;
int parent_dfd = _ostree_fetcher_get_dfd (self->metalink->fetcher);
gs_unref_object GInputStream *instream = NULL;
gs_free char *result = NULL;
GChecksum *checksum = NULL;
result = _ostree_fetcher_request_uri_with_partial_finish ((OstreeFetcher*)src, res, &local_error);
if (!result)
goto out;
finfo = g_file_query_info (result, OSTREE_GIO_FAST_QUERYINFO, 0,
g_task_get_cancellable (task), &local_error);
if (!finfo)
goto out;
if (g_file_info_get_size (finfo) != self->size)
if (!ot_openat_read_stream (parent_dfd, result, FALSE,
&instream, NULL, &local_error))
goto out;
if (fstat (g_file_descriptor_based_get_fd ((GFileDescriptorBased*)instream), &stbuf) != 0)
{
gs_set_error_from_errno (&local_error, errno);
goto out;
}
if (stbuf.st_size != self->size)
{
g_set_error (&local_error, G_IO_ERROR, G_IO_ERROR_FAILED,
"Expected size is %" G_GUINT64_FORMAT " bytes but content is %" G_GUINT64_FORMAT " bytes",
self->size, g_file_info_get_size (finfo));
self->size, stbuf.st_size);
goto out;
}
if (self->verification_sha512)
{
gs_free char *actual = ot_checksum_file (result, G_CHECKSUM_SHA512,
g_task_get_cancellable (task),
&local_error);
if (!actual)
const char *actual;
checksum = g_checksum_new (G_CHECKSUM_SHA512);
if (!ot_gio_splice_update_checksum (NULL, instream, checksum,
g_task_get_cancellable (task),
&local_error))
goto out;
actual = g_checksum_get_string (checksum);
if (strcmp (self->verification_sha512, actual) != 0)
{
@ -466,16 +479,19 @@ on_fetched_url (GObject *src,
goto out;
}
}
if (self->verification_sha256)
else if (self->verification_sha256)
{
gs_free char *actual = ot_checksum_file (result, G_CHECKSUM_SHA256,
g_task_get_cancellable (task),
&local_error);
if (!actual)
const char *actual;
checksum = g_checksum_new (G_CHECKSUM_SHA256);
if (!ot_gio_splice_update_checksum (NULL, instream, checksum,
g_task_get_cancellable (task),
&local_error))
goto out;
actual = g_checksum_get_string (checksum);
if (strcmp (self->verification_sha256, actual) != 0)
{
g_set_error (&local_error, G_IO_ERROR, G_IO_ERROR_FAILED,
@ -486,6 +502,8 @@ on_fetched_url (GObject *src,
}
out:
if (checksum)
g_checksum_free (checksum);
if (local_error)
{
g_free (self->last_metalink_error);
@ -498,7 +516,8 @@ on_fetched_url (GObject *src,
}
else
{
self->result = g_object_ref (result);
self->result = result;
result = NULL; /* Transfer ownership */
g_task_return_boolean (self->task, TRUE);
}
}
@ -584,7 +603,7 @@ ostree_metalink_request_unref (gpointer data)
{
OstreeMetalinkRequest *request = data;
g_object_unref (request->metalink);
g_clear_object (&request->result);
g_free (request->result);
g_free (request->last_metalink_error);
g_ptr_array_unref (request->urls);
g_free (request);
@ -601,7 +620,7 @@ static const GMarkupParser metalink_parser = {
typedef struct
{
SoupURI **out_target_uri;
GFile **out_data;
char **out_data;
gboolean success;
GError **error;
GMainLoop *loop;
@ -611,7 +630,7 @@ static gboolean
ostree_metalink_request_finish (OstreeMetalink *self,
GAsyncResult *result,
SoupURI **out_target_uri,
GFile **out_data,
char **out_data,
GError **error)
{
OstreeMetalinkRequest *request;
@ -624,7 +643,7 @@ ostree_metalink_request_finish (OstreeMetalink *self,
{
g_assert_cmpint (request->current_url_index, <, request->urls->len);
*out_target_uri = request->urls->pdata[request->current_url_index];
*out_data = g_object_ref (request->result);
*out_data = g_strdup (request->result);
return TRUE;
}
else
@ -668,7 +687,7 @@ gboolean
_ostree_metalink_request_sync (OstreeMetalink *self,
GMainLoop *loop,
SoupURI **out_target_uri,
GFile **out_data,
char **out_data,
SoupURI **fetching_sync_uri,
GCancellable *cancellable,
GError **error)

View File

@ -53,7 +53,7 @@ SoupURI *_ostree_metalink_get_uri (OstreeMetalink *self);
gboolean _ostree_metalink_request_sync (OstreeMetalink *self,
GMainLoop *loop,
SoupURI **out_target_uri,
GFile **out_data,
char **out_data,
SoupURI **fetching_sync_uri,
GCancellable *cancellable,
GError **error);

View File

@ -29,11 +29,14 @@
#include "ostree-metalink.h"
#include "otutil.h"
#include <gio/gunixinputstream.h>
#define OSTREE_REPO_PULL_CONTENT_PRIORITY (OSTREE_FETCHER_DEFAULT_PRIORITY)
#define OSTREE_REPO_PULL_METADATA_PRIORITY (OSTREE_REPO_PULL_CONTENT_PRIORITY - 100)
typedef struct {
OstreeRepo *repo;
int tmpdir_dfd;
OstreeRepoPullFlags flags;
char *remote_name;
OstreeRepoMode remote_mode;
@ -569,7 +572,7 @@ content_fetch_on_complete (GObject *object,
gs_unref_variant GVariant *xattrs = NULL;
gs_unref_object GInputStream *file_in = NULL;
gs_unref_object GInputStream *object_input = NULL;
gs_unref_object GFile *temp_path = NULL;
gs_free char *temp_path = NULL;
const char *checksum;
OstreeObjectType objtype;
@ -582,12 +585,12 @@ content_fetch_on_complete (GObject *object,
g_debug ("fetch of %s complete", ostree_object_to_string (checksum, objtype));
if (!ostree_content_file_parse (TRUE, temp_path, FALSE,
&file_in, &file_info, &xattrs,
cancellable, error))
if (!ostree_content_file_parse_at (TRUE, pull_data->tmpdir_dfd, temp_path, FALSE,
&file_in, &file_info, &xattrs,
cancellable, error))
{
/* If it appears corrupted, delete it */
(void) gs_file_unlink (temp_path, NULL, NULL);
(void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0);
goto out;
}
@ -595,7 +598,7 @@ content_fetch_on_complete (GObject *object,
* a reference to the fd. If we fail to write later, then
* the temp space will be cleaned up.
*/
(void) gs_file_unlink (temp_path, NULL, NULL);
(void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0);
if (!ostree_raw_file_to_content_stream (file_in, file_info, xattrs,
&object_input, &length,
@ -677,11 +680,12 @@ meta_fetch_on_complete (GObject *object,
FetchObjectData *fetch_data = user_data;
OtPullData *pull_data = fetch_data->pull_data;
gs_unref_variant GVariant *metadata = NULL;
gs_unref_object GFile *temp_path = NULL;
gs_free char *temp_path = NULL;
const char *checksum;
OstreeObjectType objtype;
GError *local_error = NULL;
GError **error = &local_error;
gs_fd_close int fd = -1;
ostree_object_name_deserialize (fetch_data->object, &checksum, &objtype);
g_debug ("fetch of %s%s complete", ostree_object_to_string (checksum, objtype),
@ -702,14 +706,21 @@ meta_fetch_on_complete (GObject *object,
goto out;
}
fd = openat (pull_data->tmpdir_dfd, temp_path, O_RDONLY | O_CLOEXEC);
if (fd == -1)
{
gs_set_error_from_errno (error, errno);
goto out;
}
if (fetch_data->is_detached_meta)
{
if (!ot_util_variant_map (temp_path, G_VARIANT_TYPE ("a{sv}"),
FALSE, &metadata, error))
if (!ot_util_variant_map_fd (fd, 0, G_VARIANT_TYPE ("a{sv}"),
FALSE, &metadata, error))
goto out;
/* Now delete it, see comment in corresponding content fetch path */
(void) gs_file_unlink (temp_path, NULL, NULL);
(void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0);
if (!ostree_repo_write_commit_detached_metadata (pull_data->repo, checksum, metadata,
pull_data->cancellable, error))
@ -719,11 +730,11 @@ meta_fetch_on_complete (GObject *object,
}
else
{
if (!ot_util_variant_map (temp_path, ostree_metadata_variant_type (objtype),
FALSE, &metadata, error))
if (!ot_util_variant_map_fd (fd, 0, ostree_metadata_variant_type (objtype),
FALSE, &metadata, error))
goto out;
(void) gs_file_unlink (temp_path, NULL, NULL);
(void) unlinkat (pull_data->tmpdir_dfd, temp_path, 0);
/* Write the commitpartial file now while we're still fetching data */
if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
@ -797,12 +808,13 @@ static_deltapart_fetch_on_complete (GObject *object,
FetchStaticDeltaData *fetch_data = user_data;
OtPullData *pull_data = fetch_data->pull_data;
gs_unref_variant GVariant *metadata = NULL;
gs_unref_object GFile *temp_path = NULL;
gs_free char *temp_path = NULL;
gs_unref_object GInputStream *in = NULL;
gs_free char *actual_checksum = NULL;
gs_free guint8 *csum = NULL;
GError *local_error = NULL;
GError **error = &local_error;
gs_fd_close int fd = -1;
g_debug ("fetch static delta part %s complete", fetch_data->expected_checksum);
@ -810,10 +822,14 @@ static_deltapart_fetch_on_complete (GObject *object,
if (!temp_path)
goto out;
in = (GInputStream*)g_file_read (temp_path, pull_data->cancellable, error);
if (!in)
goto out;
fd = openat (pull_data->tmpdir_dfd, temp_path, O_RDONLY | O_CLOEXEC);
if (fd == -1)
{
gs_set_error_from_errno (error, errno);
goto out;
}
in = g_unix_input_stream_new (fd, FALSE);
/* TODO - consider making async */
if (!ot_gio_checksum_stream (in, &csum, pull_data->cancellable, error))
goto out;
@ -832,10 +848,14 @@ static_deltapart_fetch_on_complete (GObject *object,
(void) g_input_stream_close (in, NULL, NULL);
{
gs_unref_bytes GBytes *delta_data
= gs_file_map_readonly (temp_path, pull_data->cancellable, error);
if (!delta_data)
GMappedFile *mfile = NULL;
gs_unref_bytes GBytes *delta_data = NULL;
mfile = g_mapped_file_new_from_fd (fd, FALSE, error);
if (!mfile)
goto out;
delta_data = g_mapped_file_get_bytes (mfile);
g_mapped_file_unref (mfile);
_ostree_static_delta_part_execute_async (pull_data->repo,
fetch_data->objects,
@ -1610,8 +1630,8 @@ ostree_repo_pull_with_options (OstreeRepo *self,
if (tls_permissive)
fetcher_flags |= OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE;
pull_data->fetcher = _ostree_fetcher_new (pull_data->repo->tmp_dir,
fetcher_flags);
pull_data->tmpdir_dfd = pull_data->repo->tmp_dir_fd;
pull_data->fetcher = _ostree_fetcher_new (pull_data->tmpdir_dfd, fetcher_flags);
requested_refs_to_fetch = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
commits_to_fetch = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
@ -1711,9 +1731,10 @@ ostree_repo_pull_with_options (OstreeRepo *self,
}
else
{
gs_unref_object GFile *metalink_data = NULL;
gs_free char *metalink_data = NULL;
SoupURI *metalink_uri = soup_uri_new (metalink_url_str);
SoupURI *target_uri = NULL;
gs_fd_close int fd = -1;
if (!metalink_uri)
{
@ -1741,8 +1762,15 @@ ostree_repo_pull_with_options (OstreeRepo *self,
soup_uri_set_path (pull_data->base_uri, repo_base);
}
if (!ot_util_variant_map (metalink_data, OSTREE_SUMMARY_GVARIANT_FORMAT, FALSE,
&pull_data->summary, error))
fd = openat (pull_data->tmpdir_dfd, metalink_data, O_RDONLY | O_CLOEXEC);
if (fd == -1)
{
gs_set_error_from_errno (error, errno);
goto out;
}
if (!ot_util_variant_map_fd (fd, 0, OSTREE_SUMMARY_GVARIANT_FORMAT, FALSE,
&pull_data->summary, error))
goto out;
}

View File

@ -167,7 +167,7 @@ variant_map_data_destroy (gpointer data)
}
gboolean
ot_util_variant_map_fd (GFileDescriptorBased *stream,
ot_util_variant_map_fd (int fd,
goffset start,
const GVariantType *type,
gboolean trusted,
@ -180,12 +180,14 @@ ot_util_variant_map_fd (GFileDescriptorBased *stream,
VariantMapData *mdata = NULL;
gsize len;
if (!gs_stream_fstat (stream, &stbuf, NULL, error))
goto out;
if (fstat (fd, &stbuf) != 0)
{
gs_set_error_from_errno (error, errno);
goto out;
}
len = stbuf.st_size - start;
map = mmap (NULL, len, PROT_READ, MAP_PRIVATE,
g_file_descriptor_based_get_fd (stream), start);
map = mmap (NULL, len, PROT_READ, MAP_PRIVATE, fd, start);
if (!map)
{
gs_set_error_from_errno (error, errno);

View File

@ -48,7 +48,7 @@ gboolean ot_util_variant_map (GFile *src,
GVariant **out_variant,
GError **error);
gboolean ot_util_variant_map_fd (GFileDescriptorBased *stream,
gboolean ot_util_variant_map_fd (int fd,
goffset offset,
const GVariantType *type,
gboolean trusted,