Linux Test Project
 help / color / mirror / Atom feed
From: linuxtestproject.agent@gmail.com
To: Sachin Sant <sachinp@linux.ibm.com>
Cc: ltp@lists.linux.it
Subject: Re: [LTP] fs/acl: Add ACL_USER_OBJ permission test
Date: Fri, 12 Jun 2026 19:13:35 +0000	[thread overview]
Message-ID: <20260612191335.4231-1-linuxtestproject.agent@gmail.com> (raw)
In-Reply-To: <20260612171930.11964-2-sachinp@linux.ibm.com>

Hi Sachin,

On Fri, 12 Jun 2026, Sachin Sant wrote:
> fs/acl: Add ACL_USER_OBJ permission test

--- [PATCH 1/8] ---

> +static inline void reset_test_path_no_chown(void)
> +{
> +	SAFE_TRY_UNLINK(TESTSYMLINK);
> +	SAFE_TRY_UNLINK(TESTFILE);
> +	SAFE_TRY_RMDIR(TESTDIR);
> +
> +	SAFE_MKDIR(TESTDIR, 0755);
> +}
> +
> +static inline void reset_test_path(void)
> +{
> +	reset_test_path_no_chown();
> +}

reset_test_path() and reset_test_path_no_chown() are identical.
The "_no_chown" suffix implies that reset_test_path() performs a
chown, but it does not. Could this cause confusion for future
maintainers?

If both need to exist for readability, the naming should reflect
the actual difference (which is currently none). Otherwise, one
of them could be dropped.

> +/*\
> + * Test ACL_USER_OBJ permissions using direct xattr manipulation.
> + *
> + * Verify that owner permissions (ACL_USER_OBJ) correctly control access
> + * to files and directories. The test validates that:
> + * - ACL_USER_OBJ permissions are applied directly as the owner bits
> + * - Setting ACL_USER_OBJ=rwx via setxattr() overrides a previous
> + *   chmod restriction

setxattr() should use the RST manpage role for the test catalog,
e.g. :manpage:`setxattr(2)`.

--- [PATCH 5/8] ---

> +/*\
> + * Test chmod and chown interaction with ACLs using direct xattr manipulation.
> + *
> + * Verify that standard file operations (chmod, chown) interact correctly
> + * with ACLs:
> + * - chmod should update ACL_USER_OBJ, ACL_GROUP_OBJ, and ACL_OTHER entries
> + * - chown should change file owner/group without affecting ACL entries

Since the doc comment block is exported to the test catalog, chmod
and chown should use the RST manpage role, e.g.
:manpage:`chmod(2)` and :manpage:`chown(2)`.

> +static struct tst_test test = {
> +	.test = run,
> +	.tcnt = 2,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.needs_root = 1,
> +	.mount_device = 1,
> +	.mntpoint = MNTPOINT,
> +	.forks_child = 1,

This test never calls SAFE_FORK() or any forking helper. Is
.forks_child = 1 needed here?

Verdict - Needs revision

---
Note:

The agent can sometimes produce false positives although often its
findings are genuine. If you find issues with the review, please
comment this email or ignore the suggestions.

Regards,
LTP AI Reviewer

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

  reply	other threads:[~2026-06-12 19:13 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-12 17:19 [LTP] [PATCH v7 0/8] Convert shell-based ACL test (tacl_xattr.sh) to C Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 1/8] fs/acl: Add ACL_USER_OBJ permission test Sachin Sant
2026-06-12 19:13   ` linuxtestproject.agent [this message]
2026-06-12 17:19 ` [LTP] [PATCH v7 2/8] fs/acl: Add ACL mask interaction tests Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 3/8] fs/acl: Add ACL_OTHER permission test Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 4/8] fs/acl: Add default ACL inheritance test Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 5/8] fs/acl: Add chmod/chown ACL interaction tests Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 6/8] fs/acl: Add ACL symlink operations test Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 7/8] fs/acl: Add extended attributes test Sachin Sant
2026-06-12 17:19 ` [LTP] [PATCH v7 8/8] fs/acl: Remove old shell-based ACL test Sachin Sant
  -- strict thread matches above, loose matches on Subject: below --
2026-06-16  4:54 [LTP] [PATCH v12 1/8] fs/acl: Add ACL_USER_OBJ permission test Sachin Sant
2026-06-16  8:39 ` [LTP] " linuxtestproject.agent
2026-06-15 15:15 [LTP] [PATCH v11 1/8] " Sachin Sant
2026-06-15 18:35 ` [LTP] " linuxtestproject.agent
2026-06-15  7:25 [LTP] [PATCH v10 1/8] " Sachin Sant
2026-06-15  8:33 ` [LTP] " linuxtestproject.agent
2026-06-15  9:04   ` Andrea Cervesato via ltp
2026-06-15  9:29     ` Andrea Cervesato via ltp
2026-06-15 11:25     ` Sachin Sant
2026-06-15 12:07       ` Andrea Cervesato via ltp
2026-06-15  5:29 [LTP] [PATCH v9 1/8] " Sachin Sant
2026-06-15  6:32 ` [LTP] " linuxtestproject.agent
2026-06-13  9:05 [LTP] [PATCH v8 1/8] " Sachin Sant
2026-06-13 10:36 ` [LTP] " linuxtestproject.agent
2026-06-12 10:24 [LTP] [PATCH v6 1/8] " Sachin Sant
2026-06-12 12:27 ` [LTP] " linuxtestproject.agent
2026-06-08  9:21 [LTP] [PATCH v5 1/8] " Sachin Sant
2026-06-08 11:01 ` [LTP] " linuxtestproject.agent

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260612191335.4231-1-linuxtestproject.agent@gmail.com \
    --to=linuxtestproject.agent@gmail.com \
    --cc=ltp@lists.linux.it \
    --cc=sachinp@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox