From fa35d059daddebb6afc4e4be698b27223297f788 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Fri, 14 Apr 2023 13:53:41 +1000 Subject: [PATCH] Re-write remove_old_tsversions and greatest_version Stop deliberately breaking const rules by copying file->name into dirbuf and truncating it there. Handle files located in the root directory properly. Use unlinkat() from POSIX 200809. (cherry picked from commit 9fcd42c6726fb75681d2359913af58a4a4280df3) --- lib/isc/log.c | 191 +++++++++++++++++++++++++------------------------- 1 file changed, 95 insertions(+), 96 deletions(-) diff --git a/lib/isc/log.c b/lib/isc/log.c index ca9dc8af71..523fd2d6bb 100644 --- a/lib/isc/log.c +++ b/lib/isc/log.c @@ -20,9 +20,11 @@ #include #include /* dev_t FreeBSD 2.1 */ #include +#include #include #include +#include #include #include #include @@ -1019,25 +1021,37 @@ sync_highest_level(isc_log_t *lctx, isc_logconfig_t *lcfg) { static isc_result_t greatest_version(isc_logfile_t *file, int versions, int *greatestp) { - char *bname, *digit_end; - const char *dirname; + char *digit_end; + char dirbuf[PATH_MAX + 1]; + const char *bname; + const char *dirname = "."; int version, greatest = -1; - size_t bnamelen; isc_dir_t dir; isc_result_t result; - char sep = '/'; + size_t bnamelen; - /* - * It is safe to DE_CONST the file.name because it was copied - * with isc_mem_strdup(). - */ - bname = strrchr(file->name, sep); + bname = strrchr(file->name, '/'); if (bname != NULL) { - *bname++ = '\0'; - dirname = file->name; + /* + * Copy the complete file name to dirbuf. + */ + size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf)); + if (len >= sizeof(dirbuf)) { + result = ISC_R_NOSPACE; + syslog(LOG_ERR, "unable to remove log files: %s", + isc_result_totext(result)); + return (result); + } + + /* + * Truncate after trailing '/' so the code works for + * files in the root directory. + */ + bname++; + dirbuf[bname - file->name] = '\0'; + dirname = dirbuf; } else { - DE_CONST(file->name, bname); - dirname = "."; + bname = file->name; } bnamelen = strlen(bname); @@ -1048,7 +1062,7 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) { * Return if the directory open failed. */ if (result != ISC_R_SUCCESS) { - goto out; + return (result); } while (isc_dir_read(&dir) == ISC_R_SUCCESS) { @@ -1062,26 +1076,23 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) { * Remove any backup files that exceed versions. */ if (*digit_end == '\0' && version >= versions) { - char rmfile[PATH_MAX + 1]; - int n = snprintf(rmfile, sizeof(rmfile), - "%s/%s", dirname, - dir.entry.name); - if (n >= (int)sizeof(rmfile) || n < 0) { - result = ISC_R_NOSPACE; - syslog(LOG_ERR, - "unable to remove log files: %s", - isc_result_totext(result)); - break; - } - result = isc_file_remove(rmfile); - if (result != ISC_R_SUCCESS && - result != ISC_R_NOTFOUND) - { - syslog(LOG_ERR, - "unable to remove log file " - "'%s': %s", - rmfile, - isc_result_totext(result)); + int n = unlinkat(dirfd(dir.handle), + dir.entry.name, 0); + if (n < 0) { + result = isc_errno_toresult(errno); + if (result != ISC_R_SUCCESS && + result != ISC_R_FILENOTFOUND) + { + syslog(LOG_ERR, + "unable to remove log " + "file '%s%s': %s", + bname == file->name + ? "" + : dirname, + dir.entry.name, + isc_result_totext( + result)); + } } } else if (*digit_end == '\0' && version > greatest) { greatest = version; @@ -1091,16 +1102,7 @@ greatest_version(isc_logfile_t *file, int versions, int *greatestp) { isc_dir_close(&dir); *greatestp = greatest; - result = ISC_R_SUCCESS; - -out: - /* - * Replace the file separator if it was taken out. - */ - if (bname != file->name) { - *(bname - 1) = sep; - } - return (result); + return (ISC_R_SUCCESS); } static void @@ -1120,7 +1122,8 @@ insert_sort(int64_t to_keep[], int64_t versions, int64_t version) { } static int64_t -last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) { +last_to_keep(int64_t versions, isc_dir_t *dirp, const char *bname, + size_t bnamelen) { int64_t to_keep[ISC_LOG_MAX_VERSIONS] = { 0 }; int64_t version = 0; @@ -1163,25 +1166,37 @@ last_to_keep(int64_t versions, isc_dir_t *dirp, char *bname, size_t bnamelen) { static isc_result_t remove_old_tsversions(isc_logfile_t *file, int versions) { - isc_result_t result; - char *bname = NULL, *digit_end = NULL; - const char *dirname = NULL; + char *digit_end; + char dirbuf[PATH_MAX + 1]; + const char *bname; + const char *dirname = "."; int64_t version, last = INT64_MAX; - size_t bnamelen; isc_dir_t dir; - char sep = '/'; + isc_result_t result; + size_t bnamelen; - /* - * It is safe to DE_CONST the file.name because it was copied - * with isc_mem_strdup(). - */ - bname = strrchr(file->name, sep); + bname = strrchr(file->name, '/'); if (bname != NULL) { - *bname++ = '\0'; - dirname = file->name; + /* + * Copy the complete file name to dirbuf. + */ + size_t len = strlcpy(dirbuf, file->name, sizeof(dirbuf)); + if (len >= sizeof(dirbuf)) { + result = ISC_R_NOSPACE; + syslog(LOG_ERR, "unable to remove log files: %s", + isc_result_totext(result)); + return (result); + } + + /* + * Truncate after trailing '/' so the code works for + * files in the root directory. + */ + bname++; + dirbuf[bname - file->name] = '\0'; + dirname = dirbuf; } else { - DE_CONST(file->name, bname); - dirname = "."; + bname = file->name; } bnamelen = strlen(bname); @@ -1192,61 +1207,45 @@ remove_old_tsversions(isc_logfile_t *file, int versions) { * Return if the directory open failed. */ if (result != ISC_R_SUCCESS) { - goto out; + return (result); } last = last_to_keep(versions, &dir, bname, bnamelen); - /* - * Then we remove all files that we don't want to_keep - */ while (isc_dir_read(&dir) == ISC_R_SUCCESS) { if (dir.entry.length > bnamelen && strncmp(dir.entry.name, bname, bnamelen) == 0 && dir.entry.name[bnamelen] == '.') { - char *ename = &dir.entry.name[bnamelen + 1]; - version = strtoull(ename, &digit_end, 10); + version = strtoull(&dir.entry.name[bnamelen + 1], + &digit_end, 10); /* * Remove any backup files that exceed versions. */ if (*digit_end == '\0' && version < last) { - char rmfile[PATH_MAX + 1]; - int n = snprintf(rmfile, sizeof(rmfile), - "%s/%s", dirname, - dir.entry.name); - if (n >= (int)sizeof(rmfile) || n < 0) { - result = ISC_R_NOSPACE; - syslog(LOG_ERR, - "unable to remove log files: %s", - isc_result_totext(result)); - break; - } - result = isc_file_remove(rmfile); - if (result != ISC_R_SUCCESS && - result != ISC_R_NOTFOUND) - { - syslog(LOG_ERR, - "unable to remove log file " - "'%s': %s", - rmfile, - isc_result_totext(result)); + int n = unlinkat(dirfd(dir.handle), + dir.entry.name, 0); + if (n < 0) { + result = isc_errno_toresult(errno); + if (result != ISC_R_SUCCESS && + result != ISC_R_FILENOTFOUND) + { + syslog(LOG_ERR, + "unable to remove log " + "file '%s%s': %s", + bname == file->name + ? "" + : dirname, + dir.entry.name, + isc_result_totext( + result)); + } } } } } - isc_dir_close(&dir); - result = ISC_R_SUCCESS; - -out: - /* - * Replace the file separator if it was taken out. - */ - if (bname != file->name) { - *(bname - 1) = sep; - } - return (result); + return (ISC_R_SUCCESS); } static isc_result_t