Hi! On 05/15/2013 04:38 PM, [email protected] wrote: > Hi! >> +++ b/testcases/kernel/controllers/cgroup_xattr/cgroup_xattr.c >> @@ -0,0 +1,343 @@ >> +/* >> + * Copyright (c) 2013 Oracle and/or its affiliates. All Rights Reserved. >> + * >> + * This program is free software; you can redistribute it and/or >> + * modify it under the terms of the GNU General Public License as >> + * published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it would be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + * >> + * You should have received a copy of the GNU General Public License >> + * along with this program; if not, write the Free Software Foundation, >> + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA >> + * >> + * Author: >> + * Alexey Kodanev<[email protected]> >> + * >> + * Test checks following preconditions: >> + * since Linux kernel 3.7 it is possible to set extended attributes >> + * to cgroup files. >> + */ >> + >> +#include<sys/stat.h> >> +#include<sys/mount.h> >> +#include<sys/types.h> >> +#include<sys/xattr.h> >> +#include<stdio.h> >> +#include<stdlib.h> >> +#include<fcntl.h> >> +#include<dirent.h> >> +#include<unistd.h> >> +#include<string.h> >> +#include<errno.h> >> + >> +#include "test.h" >> +#include "usctest.h" >> +#include "safe_macros.h" >> + >> +char *TCID = "cgroup_xattr"; >> + >> +static const char cgrp_point[] = "/sys/fs/cgroup"; >> +static const char cgrp_name[] = "cgrp_test"; >> +static const char subdir_name[] = "test"; >> + >> +struct tst_key { >> + const char *name; >> + int good; >> +}; >> + >> +/* only security.*& trusted.* is valid key names */ > ^ are >> +static const struct tst_key tkeys[] = { >> + { .name = "trusted.test", .good = 1, }, >> + { .name = "security.", .good = 1, }, >> + { .name = "user.", .good = 0, }, >> + { .name = "system.", .good = 0, }, >> +}; >> + >> +#define VALUE_SIZE 8 >> + >> +/* >> + * values that can be written to xattr keys, >> + * their can be anything > ^ their value? Yes >> + */ >> +struct tst_val { >> + char buf[VALUE_SIZE]; >> + size_t size; >> +}; >> + >> +/* cleanup flags */ >> +static int cgrp_mounted; >> +static int subdir_created; >> + >> +/* test options */ >> +static int skip_cleanup; >> +static int verbose; >> +static const option_t options[] = { >> + {"s",&skip_cleanup, NULL}, >> + {"v",&verbose, NULL}, >> + {NULL, NULL, NULL} >> +}; >> + >> +/* save to change back in the end */ >> +static char start_work_dir[PATH_MAX]; >> + >> +static void help(void); >> +static void setup(int argc, char *argv[]); >> +static void test_run(void); >> +static void cleanup(void); >> + >> +static int set_xattrs(const char *file, const struct tst_val *val); >> +static char *get_xattr(const char *file, const char *key_name, size_t >> *size); >> +static int get_xattrs(const char *file, const struct tst_val *val); >> +/* >> + * set or get xattr recursively >> + * >> + * @path: start directory >> + * @id: start char to fill in value >> + * @xattr_operation: can be set_xattrs() or get_xattrs() >> + */ >> +static int cgrp_files_walking(const char *path, char *id, >> + int (*xattr_operation)(const char *, const struct tst_val *)); >> +static char *get_hex_value(const char *value, size_t size); >> +static void fill_test_value(char *id, struct tst_val *val); >> + >> + >> +int main(int argc, char *argv[]) >> +{ >> + setup(argc, argv); >> + >> + test_run(); >> + >> + cleanup(); >> + >> + tst_exit(); >> +} >> + >> +static void help(void) >> +{ >> + printf(" -s Skip cleanup\n"); >> + printf(" -v Verbose\n"); >> +} >> + >> +void setup(int argc, char *argv[]) >> +{ >> + char *msg; >> + msg = parse_opts(argc, argv, options, help); >> + if (msg != NULL) >> + tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg); >> + >> + tst_require_root(NULL); >> + >> + if (access("/proc/cgroups", F_OK) == -1) >> + tst_brkm(TCONF, cleanup, "Kernel doesn't support for cgroups"); >> + >> + if (tst_kvercmp(3, 7, 0)< 0) { >> + tst_brkm(TCONF, cleanup, >> + "Test must be run with kernel 3.7 or newer"); >> + } >> + >> + tst_sig(FORK, DEF_HANDLER, cleanup); >> + >> + /* mount all available subsystems (cpu, cpuset, memory, ...) */ >> + if (mount(cgrp_name, cgrp_point, "cgroup", 0, "xattr") == -1) >> + tst_brkm(TBROK, cleanup, "Can't mount: %s", cgrp_point); > Here we mount cgroups under /sys/fs/cgroup which later causes problems > as you need to switch to another directory to unmount. > > I'm not that much familiar with cgroups, is mounting it to /sys/ > required or is it customary? Wouldn't that interfere with rest of the > system? It's standard, but in the test better use another one. > What about calling tst_tmpdir(), creating a directory to mount the > cgroups there and then simply doing chdir to test temporary directory > before the unmount. Would that work? Yes, it's possible, I'll change it to tmp dir. >> + cgrp_mounted = 1; >> + >> + /* save current working directory */ >> + SAFE_GETCWD(cleanup, start_work_dir, PATH_MAX); >> + SAFE_CHDIR(cleanup, cgrp_point); >> + >> + /* create new hierarchy */ >> + SAFE_MKDIR(cleanup, subdir_name, 0755); >> + subdir_created = 1; >> +} >> + >> +static void test_run() > void in params is missing > >> +{ >> + char id; >> + /* set xattr to each file in cgroup fs */ >> + id = 0; >> + int set_res = cgrp_files_walking(cgrp_point,&id, set_xattrs); > Here you are passing pointer to the value, modify it by the function but > never use the resulting value. > >> + /* get& check xattr from each file in cgroup fs */ >> + id = 0; >> + int get_res = cgrp_files_walking(cgrp_point,&id, get_xattrs); >> + >> + /* final results */ >> + tst_resm(TINFO, "All test-cases have been completed, summary:"); >> + tst_resm(TINFO, "Set tests result: %s", (set_res) ? "FAIL" : "PASS"); >> + tst_resm(TINFO, "Get tests result: %s", (get_res) ? "FAIL" : "PASS"); >> +} >> + >> +static void cleanup(void) >> +{ >> + if (skip_cleanup) >> + return; >> + >> + fflush(stdout); > What is this fflush() for? > kernels 3.7 can crash while unmounting cgroups with xattr, so just to make sure all buffered data written before it happens >> + if (subdir_created) { >> + SAFE_CHDIR(NULL, cgrp_point); >> + if (rmdir(subdir_name) == -1) { >> + tst_brkm(TBROK, NULL, "Can't remove dir, error: %s", >> + strerror(errno)); >> + } >> + } >> + >> + if (strlen(start_work_dir)> 0) >> + SAFE_CHDIR(NULL, start_work_dir); >> + >> + if (cgrp_mounted) { >> + if (umount(cgrp_point) == -1) >> + tst_brkm(TBROK, NULL, "Can't unmount: %s", cgrp_point); >> + } >> + >> + TEST_CLEANUP; >> +} >> + >> +static int set_xattrs(const char *file, const struct tst_val *val) >> +{ >> + int i, res; >> + res = 0; >> + for (i = 0; i< ARRAY_SIZE(tkeys); ++i) { >> + int good = setxattr(file, tkeys[i].name, >> + (const void *)val->buf, val->size, 0) != -1; >> + >> + int fail = good != tkeys[i].good; >> + res |= fail; >> + >> + tst_resm((fail) ? TFAIL : TPASS, >> + "Expect: %s set xattr key '%s' to file '%s'", >> + (tkeys[i].good) ? "can" : "can't", >> + tkeys[i].name, file); >> + >> + if (verbose&& tkeys[i].good) { >> + char *rval = get_hex_value(val->buf, val->size); >> + tst_resm(TINFO, "value =%s", rval); >> + free(rval); >> + } >> + } >> + return res; >> +} >> + >> +static char *get_xattr(const char *file, const char *key_name, size_t *size) >> +{ >> + char *xval = NULL; >> + /* get value size */ >> + ssize_t xval_size = getxattr(file, key_name, (void *)xval, 0); > > Pass NULL here, instead of the xval initialized to NULL. Ok >> + if (xval_size == -1) >> + return NULL; >> + >> + xval = SAFE_MALLOC(cleanup, xval_size); >> + >> + if (getxattr(file, key_name, (void *)xval, xval_size) == -1) { >> + free(xval); >> + return NULL; >> + } >> + *size = xval_size; >> + return xval; >> +} >> + >> +static int get_xattrs(const char *file, const struct tst_val *val) >> +{ >> + int i, fail, res; >> + res = 0; >> + for (i = 0; i< ARRAY_SIZE(tkeys); ++i) { >> + size_t xval_size = 0; >> + char *xval; >> + >> + xval = get_xattr(file, tkeys[i].name,&xval_size); >> + fail = (xval == NULL&& tkeys[i].good); >> + res |= fail; >> + >> + tst_resm((fail) ? TFAIL : TPASS, >> + "Expect: %s read xattr %s of file '%s'", >> + (xval == NULL) ? "can't" : "can", tkeys[i].name, file); >> + if (xval == NULL) >> + continue; >> + >> + if (fail) { >> + free(xval); >> + continue; >> + } >> + >> + fail |= val->size != xval_size || >> + strncmp(val->buf, xval, val->size) != 0; >> + res |= fail; >> + >> + tst_resm((fail) ? TFAIL : TPASS, "Expect: values equal"); >> + >> + if (verbose&& fail) { >> + char *rval = get_hex_value(xval, xval_size); >> + tst_resm(TINFO, "Read xattr value:%s", rval); >> + free(rval); >> + char *cval = get_hex_value(val->buf, val->size); >> + tst_resm(TINFO, "Expected value:%s", cval); >> + free(cval); >> + } >> + free(xval); >> + } >> + return res; >> +} >> + >> +static int cgrp_files_walking(const char *path, char *id, >> + int (*xattr_operation)(const char *, const struct tst_val *)) >> +{ >> + int res = 0; >> + struct dirent *entry; >> + DIR *dir; >> + dir = opendir(path); >> + SAFE_CHDIR(cleanup, path); >> + tst_resm(TINFO, "In dir %s", path); >> + errno = 0; >> + while ((entry = readdir(dir)) != NULL) { >> + const char *file = entry->d_name; >> + /* skip current and up directories */ >> + if (!strcmp(file, "..") || !strcmp(file, ".")) >> + continue; >> + >> + struct stat stat_buf; >> + TEST(lstat(file,&stat_buf)); >> + if (TEST_RETURN != -1&& S_ISDIR(stat_buf.st_mode)) { >> + /* proceed to subdir */ >> + res |= cgrp_files_walking(file, id, xattr_operation); >> + /* change directory back */ >> + SAFE_CHDIR(cleanup, ".."); >> + tst_resm(TINFO, "In dir %s", path); >> + } >> + struct tst_val val; >> + fill_test_value(id,&val); >> + res |= xattr_operation(file,&val); >> + errno = 0; >> + } >> + if (errno&& !entry) >> + tst_brkm(TWARN, cleanup, "%s", strerror(errno)); > Use TERRNO and also describe the warning more verbosely. Ok >> + if (closedir(dir) == -1) >> + tst_brkm(TWARN, cleanup, "Failed to close dir '%s'", path); >> + >> + return res; >> +} >> + >> +static char *get_hex_value(const char *value, size_t size) >> +{ >> + const size_t symb_num = 5; /* space + 0xXX*/ >> + char *buf = SAFE_MALLOC(cleanup, size * symb_num + 1); >> + size_t i; >> + for (i = 0; i< size; ++i) { >> + sprintf(buf + i * symb_num, " 0x%02X", >> + (unsigned char) *(value++)); >> + } >> + return buf; >> +} > This function is too ugly. You should rather do something like > print_xattr() that would both preprare the string and call the > tst_resm() so that you don't need to pass allocated buffers around. > > void print_xattr(const char *msg, const char *val, size_t size) > { > char buf[size + sybm_num + 1]; > ... > > tst_resm(TINFO, "%s%s", msg, buf); > } > > Or we can create a tst_xxx function to print a buffer of bytes, I guess > that there are more cases where such function could be used. But that > would require a little more work to desing the interface right. > I agree, let's do that >> +static void fill_test_value(char *id, struct tst_val *val) >> +{ >> + int i; >> + for (i = 0; i< VALUE_SIZE; ++i) >> + val->buf[i] = *id; > What about using memset()? Ok :) further may be remove this function completely and just use memset instead? >> + val->size = VALUE_SIZE; >> + ++(*id); > This is quite cryptic, why aren't you modifying the id in the loop that > calls this function? > > What about just defining the id in the walk functions and incrementing > it each time fill_test_value() was called? > The point is to make unique key's values among all hierarchies (there was a bug when files with the same name share the same struct) Would it be better to use global id that is incrementing each time when fill_test_value() called, and resetting to init value between set and get walk functions? >> +}
------------------------------------------------------------------------------ AlienVault Unified Security Management (USM) platform delivers complete security visibility with the essential security capabilities. Easily and efficiently configure, manage, and operate all of your security controls from a single console and one unified framework. Download a free trial. http://p.sf.net/sfu/alienvault_d2d _______________________________________________ Ltp-list mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ltp-list
