linux-kernel - Re: 2.6.22-rc6-mm1: UNION_FS=y + BLOCK=n = compile error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 8 Jul 2007 07:58:06 -0400
From:	Erez Zadok <ezk@...sunysb.edu>
To:	Adrian Bunk <bunk@...sta.de>
Cc:	Andrew Morton <akpm@...ux-foundation.org>, ezk@...sunysb.edu,
	jsipek@...sunysb.edu, linux-kernel@...r.kernel.org,
	unionfs@...esystems.org, axboe@...nel.dk
Subject: Re: 2.6.22-rc6-mm1: UNION_FS=y + BLOCK=n = compile error 

I'm adding Jens Axboe to the CC list (BLOCK layer maintainer).

In message <20070707020142.GA3492@...sta.de>, Adrian Bunk writes:
> On Thu, Jun 28, 2007 at 03:43:21AM -0700, Andrew Morton wrote:
> >...
> > Changes since 2.6.22-rc4-mm2:
> >...
> >  git-unionfs.patch
> >...
> >  git trees
> >...
> 
> CONFIG_UNION_FS=y, CONFIG_BLOCK=n results in the following compile error:
> 
> <--  snip  -->
> 
> ...
>   CC      fs/unionfs/file.o
> /home/bunk/linux/kernel-2.6/linux-2.6.22-rc6-mm1/fs/unionfs/file.c:147: error: â€˜file_fsyncâ€™ undeclared here (not in a function)
> make[3]: *** [fs/unionfs/file.o] Error 1
> ...
> 
> <--  snip  -->
> 
> cu
> Adrian

Adrian, thanks.  I was able to reproduce this compiler error.  But I think
there may be a problem with the mainline kernel itself.

Unionfs defines its fsync file op to file_fsync.  file_fsync, defined in
fs/sync.c, appears to be a generic syncing function that can be used by
other file systems.  In fact, the comment above it says "Generic function to
fsync a file."  From fs/Makefile, it looks like sync.c gets compiled
regardless whether CONFIG_BLOCK is defined or not.

Moreover, file_fsync() makes a call to sync_blockdev(), clearly a function
which depends on CONFIG_BLOCK=y.  However, include/linux/buffer_head.h
defines sync_blockdev() to a noop if CONFIG_BLOCK=n:

  static inline int sync_blockdev(struct block_device *bdev) { return 0; }

This further indicates to me that someone went through the effort of making
file_fsync available and operational *even* if CONFIG_BLOCK=n.

But here's the problem: if CONFIG_BLOCK=n, the function file_fsync exists
and gets compiled into the kernel, but the extern definition for it is not
compiled in (it's wrapped in an #ifdef CONFIG_BLOCK in buffer_head.h).  This
looks to me like a discrepancy: why compile the file_fsync function, and
even ensure that it will find a dummy sync_blockdev() symbol, if one doesn't
provide an extern for it?

One more oddity: the EXPORT_SYMBOL for fs/sync.c:file_fsync() is in
fs/buffer.c, and buffer.c does not get compiled if CONFIG_BLOCK=n.  I
thought all EXPORT_SYMBOL calls are supposed to be near their respective
functions, no?

I suggest one of two things:

1. If you agree with me as to this discrepancy, then we can move the extern
   for file_fsync() outside the #ifdef CONFIG_BLOCK in buffer_head.h -- it
   should be defined regardless.  Then move the EXPORT_SYMBOL(file_fsync)
   from buffer.c to sync.  I can provide a small patch right away.

2. If you disagree, then I can change Unionfs to avoid calling file_fsync
   directly.  I can define a unionfs_fsync file method which does the usual
   stuff (pass the op to the layer below).

My preference is option #1; I'd like to avoid defining any more stackable
wrappers than I have to.

Thanks,
Erez.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/