From c53f2e9b5086051a9aede27254a109983443e557 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Mon, 17 May 2021 18:13:18 +0200 Subject: [PATCH] libshare: nfs: open temporary file once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Don Brady Reviewed-by: Brian Behlendorf Reviewed-by: John Kennedy Signed-off-by: Ahelenia Ziemiańska Closes #12067 --- lib/libshare/nfs.c | 83 +++++++++++++++++++++++------------ lib/libshare/nfs.h | 4 +- lib/libshare/os/freebsd/nfs.c | 43 ++++-------------- lib/libshare/os/linux/nfs.c | 57 +++++++----------------- lib/libshare/os/linux/smb.c | 2 +- 5 files changed, 80 insertions(+), 109 deletions(-) diff --git a/lib/libshare/nfs.c b/lib/libshare/nfs.c index 2b0c70001d..e7037b104c 100644 --- a/lib/libshare/nfs.c +++ b/lib/libshare/nfs.c @@ -77,10 +77,18 @@ nfs_exports_unlock(const char *name) nfs_lock_fd = -1; } -static char * -nfs_init_tmpfile(const char *prefix, const char *mdir) +struct tmpfile { + /* + * This only needs to be as wide as ZFS_EXPORTS_FILE and mktemp suffix, + * 64 is more than enough. + */ + char name[64]; + FILE *fp; +}; + +static boolean_t +nfs_init_tmpfile(const char *prefix, const char *mdir, struct tmpfile *tmpf) { - char *tmpfile = NULL; struct stat sb; if (mdir != NULL && @@ -88,72 +96,89 @@ nfs_init_tmpfile(const char *prefix, const char *mdir) mkdir(mdir, 0755) < 0) { fprintf(stderr, "failed to create %s: %s\n", mdir, strerror(errno)); - return (NULL); + return (B_FALSE); } - if (asprintf(&tmpfile, "%s.XXXXXXXX", prefix) == -1) { - fprintf(stderr, "Unable to allocate temporary file\n"); - return (NULL); - } + strcpy(tmpf->name, prefix); + strcat(tmpf->name, ".XXXXXXXX"); - int fd = mkostemp(tmpfile, O_CLOEXEC); + int fd = mkostemp(tmpf->name, O_CLOEXEC); if (fd == -1) { fprintf(stderr, "Unable to create temporary file: %s", strerror(errno)); - free(tmpfile); - return (NULL); + return (B_FALSE); } - close(fd); - return (tmpfile); + + tmpf->fp = fdopen(fd, "w+"); + if (tmpf->fp == NULL) { + fprintf(stderr, "Unable to reopen temporary file: %s", + strerror(errno)); + close(fd); + return (B_FALSE); + } + + return (B_TRUE); +} + +static void +nfs_abort_tmpfile(struct tmpfile *tmpf) +{ + unlink(tmpf->name); + fclose(tmpf->fp); } static int -nfs_fini_tmpfile(const char *exports, char *tmpfile) +nfs_fini_tmpfile(const char *exports, struct tmpfile *tmpf) { - if (rename(tmpfile, exports) == -1) { - fprintf(stderr, "Unable to rename %s: %s\n", tmpfile, + if (fflush(tmpf->fp) != 0) { + fprintf(stderr, "Failed to write to temporary file: %s\n", strerror(errno)); - unlink(tmpfile); - free(tmpfile); + nfs_abort_tmpfile(tmpf); return (SA_SYSTEM_ERR); } - free(tmpfile); + + if (rename(tmpf->name, exports) == -1) { + fprintf(stderr, "Unable to rename %s -> %s: %s\n", + tmpf->name, exports, strerror(errno)); + nfs_abort_tmpfile(tmpf); + return (SA_SYSTEM_ERR); + } + + fclose(tmpf->fp); return (SA_OK); } int nfs_toggle_share(const char *lockfile, const char *exports, const char *expdir, sa_share_impl_t impl_share, - int(*cbk)(sa_share_impl_t impl_share, char *filename)) + int(*cbk)(sa_share_impl_t impl_share, FILE *tmpfile)) { int error; - char *filename; + struct tmpfile tmpf; - if ((filename = nfs_init_tmpfile(exports, expdir)) == NULL) + if (!nfs_init_tmpfile(exports, expdir, &tmpf)) return (SA_SYSTEM_ERR); error = nfs_exports_lock(lockfile); if (error != 0) { - unlink(filename); - free(filename); + nfs_abort_tmpfile(&tmpf); return (error); } - error = nfs_copy_entries(filename, impl_share->sa_mountpoint); + error = nfs_copy_entries(tmpf.fp, impl_share->sa_mountpoint); if (error != SA_OK) goto fullerr; - error = cbk(impl_share, filename); + error = cbk(impl_share, tmpf.fp); if (error != SA_OK) goto fullerr; - error = nfs_fini_tmpfile(exports, filename); + error = nfs_fini_tmpfile(exports, &tmpf); nfs_exports_unlock(lockfile); return (error); fullerr: - unlink(filename); - free(filename); + nfs_abort_tmpfile(&tmpf); nfs_exports_unlock(lockfile); return (error); } diff --git a/lib/libshare/nfs.h b/lib/libshare/nfs.h index 4dbcdf5985..370c409bec 100644 --- a/lib/libshare/nfs.h +++ b/lib/libshare/nfs.h @@ -30,7 +30,7 @@ void libshare_nfs_init(void); -int nfs_copy_entries(char *filename, const char *mountpoint); +int nfs_copy_entries(FILE *tmpfile, const char *mountpoint); int nfs_toggle_share(const char *lockfile, const char *exports, const char *expdir, sa_share_impl_t impl_share, - int(*cbk)(sa_share_impl_t impl_share, char *filename)); + int(*cbk)(sa_share_impl_t impl_share, FILE *tmpfile)); diff --git a/lib/libshare/os/freebsd/nfs.c b/lib/libshare/os/freebsd/nfs.c index 8abc3bf823..0820298389 100644 --- a/lib/libshare/os/freebsd/nfs.c +++ b/lib/libshare/os/freebsd/nfs.c @@ -144,23 +144,16 @@ translate_opts(const char *shareopts) } /* - * This function copies all entries from the exports file to "filename", + * This function copies all entries from the exports file to newfp, * omitting any entries for the specified mountpoint. */ int -nfs_copy_entries(char *filename, const char *mountpoint) +nfs_copy_entries(FILE *newfp, const char *mountpoint) { int error = SA_OK; char *line; FILE *oldfp = fopen(ZFS_EXPORTS_FILE, "re"); - FILE *newfp = fopen(filename, "w+e"); - if (newfp == NULL) { - fprintf(stderr, "failed to open %s file: %s", filename, - strerror(errno)); - fclose(oldfp); - return (SA_SYSTEM_ERR); - } fputs(FILE_HEADER, newfp); /* @@ -175,47 +168,27 @@ nfs_copy_entries(char *filename, const char *mountpoint) } if (fclose(oldfp) != 0) { fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); + ZFS_EXPORTS_FILE, strerror(errno)); error = error != 0 ? error : SA_SYSTEM_ERR; } } - if (error == 0 && ferror(newfp) != 0) { + if (error == SA_OK && ferror(newfp) != 0) error = ferror(newfp); - } - if (fclose(newfp) != 0) { - fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); - error = error != 0 ? error : SA_SYSTEM_ERR; - } return (error); } static int -nfs_enable_share_impl(sa_share_impl_t impl_share, char *filename) +nfs_enable_share_impl(sa_share_impl_t impl_share, FILE *tmpfile) { - FILE *fp = fopen(filename, "a+e"); - if (fp == NULL) { - fprintf(stderr, "failed to open %s file: %s", filename, - strerror(errno)); - return (SA_SYSTEM_ERR); - } - char *shareopts = FSINFO(impl_share, nfs_fstype)->shareopts; if (strcmp(shareopts, "on") == 0) shareopts = ""; - if (fprintf(fp, "%s\t%s\n", impl_share->sa_mountpoint, + if (fprintf(tmpfile, "%s\t%s\n", impl_share->sa_mountpoint, translate_opts(shareopts)) < 0) { - fprintf(stderr, "failed to write to %s\n", filename); - fclose(fp); - return (SA_SYSTEM_ERR); - } - - if (fclose(fp) != 0) { - fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); + fprintf(stderr, "failed to write to temporary file\n"); return (SA_SYSTEM_ERR); } @@ -231,7 +204,7 @@ nfs_enable_share(sa_share_impl_t impl_share) } static int -nfs_disable_share_impl(sa_share_impl_t impl_share, char *filename) +nfs_disable_share_impl(sa_share_impl_t impl_share, FILE *tmpfile) { return (SA_OK); } diff --git a/lib/libshare/os/linux/nfs.c b/lib/libshare/os/linux/nfs.c index 4f754aabd3..bf37291411 100644 --- a/lib/libshare/os/linux/nfs.c +++ b/lib/libshare/os/linux/nfs.c @@ -51,7 +51,7 @@ static sa_fstype_t *nfs_fstype; typedef int (*nfs_shareopt_callback_t)(const char *opt, const char *value, void *cookie); -typedef int (*nfs_host_callback_t)(const char *sharepath, const char *filename, +typedef int (*nfs_host_callback_t)(FILE *tmpfile, const char *sharepath, const char *host, const char *security, const char *access, void *cookie); /* @@ -122,7 +122,7 @@ typedef struct nfs_host_cookie_s { nfs_host_callback_t callback; const char *sharepath; void *cookie; - const char *filename; + FILE *tmpfile; const char *security; } nfs_host_cookie_t; @@ -203,7 +203,7 @@ foreach_nfs_host_cb(const char *opt, const char *value, void *pcookie) } } - error = udata->callback(udata->filename, + error = udata->callback(udata->tmpfile, udata->sharepath, host, udata->security, access, udata->cookie); @@ -226,7 +226,7 @@ foreach_nfs_host_cb(const char *opt, const char *value, void *pcookie) * Invokes a callback function for all NFS hosts that are set for a share. */ static int -foreach_nfs_host(sa_share_impl_t impl_share, char *filename, +foreach_nfs_host(sa_share_impl_t impl_share, FILE *tmpfile, nfs_host_callback_t callback, void *cookie) { nfs_host_cookie_t udata; @@ -235,7 +235,7 @@ foreach_nfs_host(sa_share_impl_t impl_share, char *filename, udata.callback = callback; udata.sharepath = impl_share->sa_mountpoint; udata.cookie = cookie; - udata.filename = filename; + udata.tmpfile = tmpfile; udata.security = "sys"; shareopts = FSINFO(impl_share, nfs_fstype)->shareopts; @@ -393,7 +393,7 @@ get_linux_shareopts(const char *shareopts, char **plinux_opts) * automatically exported upon boot or whenever the nfs server restarts. */ static int -nfs_add_entry(const char *filename, const char *sharepath, +nfs_add_entry(FILE *tmpfile, const char *sharepath, const char *host, const char *security, const char *access_opts, void *pcookie) { @@ -408,50 +408,29 @@ nfs_add_entry(const char *filename, const char *sharepath, if (linux_opts == NULL) linux_opts = ""; - FILE *fp = fopen(filename, "a+e"); - if (fp == NULL) { - fprintf(stderr, "failed to open %s file: %s", filename, - strerror(errno)); - free(linuxhost); - return (SA_SYSTEM_ERR); - } - - if (fprintf(fp, "%s %s(sec=%s,%s,%s)\n", sharepath, linuxhost, + if (fprintf(tmpfile, "%s %s(sec=%s,%s,%s)\n", sharepath, linuxhost, security, access_opts, linux_opts) < 0) { - fprintf(stderr, "failed to write to %s\n", filename); + fprintf(stderr, "failed to write to temporary file\n"); free(linuxhost); - fclose(fp); return (SA_SYSTEM_ERR); } free(linuxhost); - if (fclose(fp) != 0) { - fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); - return (SA_SYSTEM_ERR); - } return (SA_OK); } /* - * This function copies all entries from the exports file to "filename", + * This function copies all entries from the exports file to newfp, * omitting any entries for the specified mountpoint. */ int -nfs_copy_entries(char *filename, const char *mountpoint) +nfs_copy_entries(FILE *newfp, const char *mountpoint) { char *buf = NULL; size_t buflen = 0; int error = SA_OK; FILE *oldfp = fopen(ZFS_EXPORTS_FILE, "re"); - FILE *newfp = fopen(filename, "w+e"); - if (newfp == NULL) { - fprintf(stderr, "failed to open %s file: %s", filename, - strerror(errno)); - fclose(oldfp); - return (SA_SYSTEM_ERR); - } fputs(FILE_HEADER, newfp); /* @@ -482,21 +461,15 @@ nfs_copy_entries(char *filename, const char *mountpoint) } if (fclose(oldfp) != 0) { fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); + ZFS_EXPORTS_FILE, strerror(errno)); error = error != 0 ? error : SA_SYSTEM_ERR; } } - if (error == 0 && ferror(newfp) != 0) { + if (error == SA_OK && ferror(newfp) != 0) error = ferror(newfp); - } free(buf); - if (fclose(newfp) != 0) { - fprintf(stderr, "Unable to close file %s: %s\n", - filename, strerror(errno)); - error = error != 0 ? error : SA_SYSTEM_ERR; - } return (error); } @@ -504,7 +477,7 @@ nfs_copy_entries(char *filename, const char *mountpoint) * Enables NFS sharing for the specified share. */ static int -nfs_enable_share_impl(sa_share_impl_t impl_share, char *filename) +nfs_enable_share_impl(sa_share_impl_t impl_share, FILE *tmpfile) { char *shareopts, *linux_opts; int error; @@ -514,7 +487,7 @@ nfs_enable_share_impl(sa_share_impl_t impl_share, char *filename) if (error != SA_OK) return (error); - error = foreach_nfs_host(impl_share, filename, nfs_add_entry, + error = foreach_nfs_host(impl_share, tmpfile, nfs_add_entry, linux_opts); free(linux_opts); return (error); @@ -532,7 +505,7 @@ nfs_enable_share(sa_share_impl_t impl_share) * Disables NFS sharing for the specified share. */ static int -nfs_disable_share_impl(sa_share_impl_t impl_share, char *filename) +nfs_disable_share_impl(sa_share_impl_t impl_share, FILE *tmpfile) { return (SA_OK); } diff --git a/lib/libshare/os/linux/smb.c b/lib/libshare/os/linux/smb.c index 9b18848e09..312ffb97d1 100644 --- a/lib/libshare/os/linux/smb.c +++ b/lib/libshare/os/linux/smb.c @@ -254,7 +254,7 @@ smb_enable_share_one(const char *sharename, const char *sharepath) argv[5] = (char *)name; argv[6] = (char *)sharepath; argv[7] = (char *)comment; - argv[8] = "Everyone:F"; + argv[8] = (char *)"Everyone:F"; argv[9] = NULL; rc = libzfs_run_process(argv[0], argv, 0);