[PATCHES] Check for missing VFS functions in time_audit and full_audit
Jeremy Allison
jra at samba.org
Sat Apr 2 00:12:08 UTC 2016
On Fri, Apr 01, 2016 at 09:53:34AM -0700, Christof Schmitt wrote:
> On Fri, Apr 01, 2016 at 11:08:27AM +0200, Volker Lendecke wrote:
> > On Thu, Mar 31, 2016 at 10:50:47PM -0700, Christof Schmitt wrote:
> > > The goal of time_audit and full_audit is to intercept all VFS functions.
> > > Everytime a new VFS functions is added, this is easily forgotten. The
> > > attached patches add a hack to check for missing functions in developer
> > > mode. It is ugly, but i could not find a better way to achieve this.
> > >
> > > On current master it already spots missing functions:
> >
> > Oh, very cool!
> >
> > Can we make this abort please?
>
> Like the attached patches? After thinking about this, moving the code in
> a vfs helper function would make sense to duplication.
>
> The downside with aborting is that we now have to add the missing
> functions before pushing this. ;-)
Oooh. I really like this. Can I add the missing functions
and get that reviewed, then I'll push this !
:-).
> From 596156de505072a680dda95043033b3941ded8cc Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Fri, 1 Apr 2016 09:47:31 -0700
> Subject: [PATCH 1/4] vfs: Add helper to check for missing VFS functions
>
> Some VFS modules want to ensure that they implement all VFS functions.
> This helper can be used to detect missing functions in the developer
> build.
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/include/vfs.h | 3 +++
> source3/smbd/vfs.c | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/source3/include/vfs.h b/source3/include/vfs.h
> index 6ab9a7e..9360802 100644
> --- a/source3/include/vfs.h
> +++ b/source3/include/vfs.h
> @@ -1381,4 +1381,7 @@ void vfs_remove_all_fsp_extensions(struct files_struct *fsp);
> void *vfs_memctx_fsp_extension(vfs_handle_struct *handle, files_struct *fsp);
> void *vfs_fetch_fsp_extension(vfs_handle_struct *handle, files_struct *fsp);
>
> +void smb_vfs_assert_all_fns(const struct vfs_fn_pointers* fns,
> + const char *module);
> +
> #endif /* _VFS_H */
> diff --git a/source3/smbd/vfs.c b/source3/smbd/vfs.c
> index efed268..605f9ad 100644
> --- a/source3/smbd/vfs.c
> +++ b/source3/smbd/vfs.c
> @@ -316,6 +316,36 @@ void *vfs_fetch_fsp_extension(vfs_handle_struct *handle, files_struct *fsp)
>
> #undef EXT_DATA_AREA
>
> +/*
> + * Ensure this module catches all VFS functions.
> + */
> +#ifdef DEVELOPER
> +void smb_vfs_assert_all_fns(const struct vfs_fn_pointers* fns,
> + const char *module)
> +{
> + bool missing_fn = false;
> + unsigned int idx;
> + const uintptr_t *end = (const uintptr_t *)(fns + 1);
> +
> + for (idx = 0; ((const uintptr_t *)fns + idx) < end; idx++) {
> + if (*((const uintptr_t *)fns + idx) == 0) {
> + DBG_ERR("VFS function at index %d not implemented "
> + "in module %s\n", idx, module);
> + missing_fn = true;
> + }
> + }
> +
> + if (missing_fn) {
> + smb_panic("Required VFS function not implemented in module.\n");
> + }
> +}
> +#else
> +void smb_vfs_assert_all_fns(const struct vfs_fn_pointers* fns,
> + const char *module)
> +{
> +}
> +#endif
> +
> /*****************************************************************
> Generic VFS init.
> ******************************************************************/
> --
> 1.8.3.1
>
>
> From 037c5a26e88580cd9cc67cf603f6b7fa79afbd30 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Thu, 31 Mar 2016 22:30:14 -0700
> Subject: [PATCH 2/4] vfs_full_audit: Assert that all VFS functions are
> implemented
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/modules/vfs_full_audit.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/source3/modules/vfs_full_audit.c b/source3/modules/vfs_full_audit.c
> index 79c57e8..654c627 100644
> --- a/source3/modules/vfs_full_audit.c
> +++ b/source3/modules/vfs_full_audit.c
> @@ -2436,9 +2436,13 @@ static struct vfs_fn_pointers vfs_full_audit_fns = {
> static_decl_vfs;
> NTSTATUS vfs_full_audit_init(void)
> {
> - NTSTATUS ret = smb_register_vfs(SMB_VFS_INTERFACE_VERSION,
> - "full_audit", &vfs_full_audit_fns);
> -
> + NTSTATUS ret;
> +
> + smb_vfs_assert_all_fns(&vfs_full_audit_fns, "full_audit");
> +
> + ret = smb_register_vfs(SMB_VFS_INTERFACE_VERSION, "full_audit",
> + &vfs_full_audit_fns);
> +
> if (!NT_STATUS_IS_OK(ret))
> return ret;
>
> --
> 1.8.3.1
>
>
> From d147f3f528fa7a9b0e81827fe3b120229f6c2c97 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Thu, 31 Mar 2016 22:30:41 -0700
> Subject: [PATCH 3/4] vfs_time_audit: Assert that all VFS functions are
> implemented
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> source3/modules/vfs_time_audit.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/source3/modules/vfs_time_audit.c b/source3/modules/vfs_time_audit.c
> index 3bdc98b..fce76df 100644
> --- a/source3/modules/vfs_time_audit.c
> +++ b/source3/modules/vfs_time_audit.c
> @@ -2539,6 +2539,8 @@ static struct vfs_fn_pointers vfs_time_audit_fns = {
> NTSTATUS vfs_time_audit_init(void);
> NTSTATUS vfs_time_audit_init(void)
> {
> + smb_vfs_assert_all_fns(&vfs_time_audit_fns, "time_audit");
> +
> audit_timeout = (double)lp_parm_int(-1, "time_audit", "timeout",
> 10000) / 1000.0;
> return smb_register_vfs(SMB_VFS_INTERFACE_VERSION, "time_audit",
> --
> 1.8.3.1
>
>
> From 6c808769886463a9b1eae855b99f85ac6f6413c8 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Thu, 31 Mar 2016 22:31:19 -0700
> Subject: [PATCH 4/4] selftest: Load time_audit and full_audit
>
> This triggers the check for missing VFS functions in these modules.
>
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
> selftest/target/Samba3.pm | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
> index bdeebc9..f4d7403 100755
> --- a/selftest/target/Samba3.pm
> +++ b/selftest/target/Samba3.pm
> @@ -548,7 +548,7 @@ sub setup_simpleserver($$)
>
> my $simpleserver_options = "
> lanman auth = yes
> - vfs objects = xattr_tdb streams_depot
> + vfs objects = xattr_tdb streams_depot time_audit full_audit
> change notify = no
>
> [vfs_aio_fork]
> --
> 1.8.3.1
>
More information about the samba-technical
mailing list