linux-kernel - Re: [PATCH v2 0/6] macros for section name cleanup

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:	Mon, 4 May 2009 12:32:51 -0400 (EDT)
From:	Tim Abbott <tabbott@....EDU>
To:	Sam Ravnborg <sam@...nborg.org>
cc:	Linux kernel mailing list <linux-kernel@...r.kernel.org>,
	linux arch <linux-arch@...r.kernel.org>,
	Anders Kaseorg <andersk@....edu>
Subject: Re: [PATCH v2 0/6] macros for section name cleanup

On Mon, 4 May 2009, Sam Ravnborg wrote:

> I'm especially fond of the "minimal" linker script
> contained in the beginning of the file.

I like this a lot too.

> + *   /DISCARD/ : {  
> + *           EXIT_TEXT
> + *           EXIT_DATA
> + *           *(.exitcall.exit)
> + *   }

You may want to create an EXIT_CALL macro, to remove the explicit mention 
of .exitcall.exit, which is the only input section referenced directly in 
the example script.

> +#define READ_MOSTLY_DATA(align)					\
> +	. = ALIGN(align);						\
> +	*(.data.read_mostly)

Should READ_MOSTLY_DATA have an ALIGN(align) at the end as well?  If the 
goal is to isolate READ_MOSTLY_DATA from other data at the given alignment 
level, this would seem appropriate.  Something similar may apply to 
CACHELINE_ALIGNED_DATA as well.

> +/*
> + * bss (Block started by Symbol) - uninitialized data
> + * zeroed during startup
> + */
> +#define SBSS								\
> +	.sbss : AT(ADDR(.sbss) - LOAD_OFFSET) {				\
> +		*(.sbss)						\
> +		*(.scommon)						\
> +	}

I notice that s390 and powerpc reference a ".dynsbss"; does that belong 
here?

> +#ifdef CONFIG_BLK_DEV_INITRD
> +#define INIT_RAM_FS							\
> +	. = ALIGN(PAGE_SIZE);						\
> +	VMLINUX_SYMBOL(__initramfs_start) = .;				\
> +	*(.init.ramfs)							\
> +	VMLINUX_SYMBOL(__initramfs_end) = .;
> +#else
> +#define INITRAMFS
> +#endif

I think you want the #else clause to be INIT_RAM_FS here.

> +/*
> + * Definition of the high level *_SECTION macros
> + * They will fit only a subset of the architectures
> + */

Do we want a TEXT_SECTION macro here as well that looks like what appears 
above in the example script?

> +#define RW_DATA_SECTION(page_align, readmostly_align, cache_align, 
inittask_align) \
> +     . = ALIGN(PAGE_SIZE);                                           \
> +     .data : AT(ADDR(.data) - LOAD_OFFSET) {                         \
> +             DATA_DATA                                               \
> +             CONSTRUCTORS                                            \
> +             NOSAVE_DATA                                             \
> +             PAGE_ALIGNED_DATA(page_align)                           \
> +             READMOSTLY_DATA(readmostly_align)                       \
> +             CACHELINE_ALIGNED_DATA(cache_align)                     \
> +             INIT_TASK(inittask_align)                               \
> +     }       

How did you pick the order of the sections here?  I would think that to 
pack the .data section efficiently, you'd want to sort by alignment 
requirements so that e.g. all the at-least-page aligned sections are 
adjacent (INIT_TASK and the page-aligned sections are separated by some 
much smaller aligments here).  So it would like either the following or 
the following reversed:

     . = ALIGN(PAGE_SIZE);                                           \
     .data : AT(ADDR(.data) - LOAD_OFFSET) {                         \
             INIT_TASK(inittask_align)                               \
             NOSAVE_DATA                                             \
             PAGE_ALIGNED_DATA(page_align)                           \
             READMOSTLY_DATA(readmostly_align)                       \
             CACHELINE_ALIGNED_DATA(cache_align)                     \
             DATA_DATA                                               \
             CONSTRUCTORS                                            \
     }       

	-Tim Abbott
--
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/