2010-08-10 09:11:09

by Christian Dietrich

[permalink] [raw]
Subject: [PATCH 0/4] Removing dead code

Hi all!

As part of the VAMOS[0] research project at the University of
Erlangen we are looking at multiple integrity errors in linux'
configuration system.

I've been running a check on the arch/frv sourcetree for
config Items not defined in Kconfig and found 4 such cases. Sourcecode
blocks depending on these Items are not reachable from a vanilla
kernel -- dead code. I've seen such dead blocks made on purpose
e.g. while integrating new features into the kernel but generally
they're just useless.

Each of the patches in this patchset removes on such dead
config Item, I'd be glad if you consider applying them. I've been
doing deeper analysis of such issues before and can do so again but
I'm not so sure they were fastly usefull.


I wasn't able to build a vanilla kernel allyesconfig, therefore these
patches might break something. But because they are just removing dead
code it is unlikely.

Another problem i encountered was CONFIG_PREEMPT, which isn't
selectable in frv Kconfig because it doesn include
kernel/Kconfig.preempt. But there ist plenty of evidence for me, that
there is preemption support in frv.


Please keep me informed of this patch getting confirmed /
merged so we can keep track of it.

Regards

Christian Dietrich

[0] http://vamos1.informatik.uni-erlangen.de/

Christian Dietrich (4):
arch/frv: Removing dead RAMKERNEL config option
arch/frv: Removing dead HEARTBEAT config option
arch/frv: Removing dead NO_KERNEL_MSG config option
arch/frv: Removing dead DEBUG_STACK_USAGE config option

arch/frv/include/asm/bug.h | 4 ----
arch/frv/include/asm/thread_info.h | 11 -----------
arch/frv/kernel/debug-stub.c | 2 --
arch/frv/kernel/time.c | 6 ------
arch/frv/mm/init.c | 17 -----------------
5 files changed, 0 insertions(+), 40 deletions(-)


2010-08-10 09:11:19

by Christian Dietrich

[permalink] [raw]
Subject: [PATCH 1/4] arch/frv: Removing dead RAMKERNEL config option

CONFIG_RAMKERNEL doesn't exist in Kconfig, therefore removing
all references to it from the source.

Signed-off-by: Christian Dietrich <[email protected]>
---
arch/frv/mm/init.c | 17 -----------------
1 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/arch/frv/mm/init.c b/arch/frv/mm/init.c
index ed64588..73586ad 100644
--- a/arch/frv/mm/init.c
+++ b/arch/frv/mm/init.c
@@ -170,23 +170,6 @@ void __init mem_init(void)
*/
void free_initmem(void)
{
-#if defined(CONFIG_RAMKERNEL) && !defined(CONFIG_PROTECT_KERNEL)
- unsigned long start, end, addr;
-
- start = PAGE_ALIGN((unsigned long) &__init_begin); /* round up */
- end = ((unsigned long) &__init_end) & PAGE_MASK; /* round down */
-
- /* next to check that the page we free is not a partial page */
- for (addr = start; addr < end; addr += PAGE_SIZE) {
- ClearPageReserved(virt_to_page(addr));
- init_page_count(virt_to_page(addr));
- free_page(addr);
- totalram_pages++;
- }
-
- printk("Freeing unused kernel memory: %ldKiB freed (0x%lx - 0x%lx)\n",
- (end - start) >> 10, start, end);
-#endif
} /* end free_initmem() */

/*****************************************************************************/
--
1.7.0.4

2010-08-10 09:11:31

by Christian Dietrich

[permalink] [raw]
Subject: [PATCH 3/4] arch/frv: Removing dead NO_KERNEL_MSG config option

CONFIG_NO_KERNEL_MSG doesn't exist in Kconfig, therefore removing
all references to it from the source.

Signed-off-by: Christian Dietrich <[email protected]>
---
arch/frv/include/asm/bug.h | 4 ----
arch/frv/kernel/debug-stub.c | 2 --
2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/frv/include/asm/bug.h b/arch/frv/include/asm/bug.h
index 2e05450..ea6e986 100644
--- a/arch/frv/include/asm/bug.h
+++ b/arch/frv/include/asm/bug.h
@@ -19,12 +19,8 @@
*/
extern asmlinkage void __debug_bug_trap(int signr);

-#ifdef CONFIG_NO_KERNEL_MSG
-#define _debug_bug_printk()
-#else
extern void __debug_bug_printk(const char *file, unsigned line);
#define _debug_bug_printk() __debug_bug_printk(__FILE__, __LINE__)
-#endif

#define _debug_bug_trap(signr) \
do { \
diff --git a/arch/frv/kernel/debug-stub.c b/arch/frv/kernel/debug-stub.c
index 2845139..8752253 100644
--- a/arch/frv/kernel/debug-stub.c
+++ b/arch/frv/kernel/debug-stub.c
@@ -250,10 +250,8 @@ int __init console_get_baud(void)
/*
* display BUG() info
*/
-#ifndef CONFIG_NO_KERNEL_MSG
void __debug_bug_printk(const char *file, unsigned line)
{
printk("kernel BUG at %s:%d!\n", file, line);

} /* end __debug_bug_printk() */
-#endif
--
1.7.0.4

2010-08-10 09:11:46

by Christian Dietrich

[permalink] [raw]
Subject: [PATCH 4/4] arch/frv: Removing dead DEBUG_STACK_USAGE config option

CONFIG_DEBUG_STACK_USAGE doesn't exist in Kconfig, therefore removing
all references to it from the source.

Signed-off-by: Christian Dietrich <[email protected]>
---
arch/frv/include/asm/thread_info.h | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/arch/frv/include/asm/thread_info.h b/arch/frv/include/asm/thread_info.h
index 11f33ea..89ab701 100644
--- a/arch/frv/include/asm/thread_info.h
+++ b/arch/frv/include/asm/thread_info.h
@@ -83,18 +83,7 @@ register struct thread_info *__current_thread_info asm("gr15");
#define __HAVE_ARCH_THREAD_INFO_ALLOCATOR

/* thread information allocation */
-#ifdef CONFIG_DEBUG_STACK_USAGE
-#define alloc_thread_info(tsk) \
- ({ \
- struct thread_info *ret; \
- \
- ret = kzalloc(THREAD_SIZE, GFP_KERNEL); \
- \
- ret; \
- })
-#else
#define alloc_thread_info(tsk) kmalloc(THREAD_SIZE, GFP_KERNEL)
-#endif

#define free_thread_info(info) kfree(info)

--
1.7.0.4

2010-08-10 09:11:39

by Christian Dietrich

[permalink] [raw]
Subject: [PATCH 2/4] arch/frv: Removing dead HEARTBEAT config option

CONFIG_HEARTBEAT doesn't exist in Kconfig, therefore removing
all references to it from the source.

Signed-off-by: Christian Dietrich <[email protected]>
---
arch/frv/kernel/time.c | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
index 0ddbbae..e86df6c 100644
--- a/arch/frv/kernel/time.c
+++ b/arch/frv/kernel/time.c
@@ -66,12 +66,6 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)

do_timer(1);

-#ifdef CONFIG_HEARTBEAT
- static unsigned short n;
- n++;
- __set_LEDS(n);
-#endif /* CONFIG_HEARTBEAT */
-
write_sequnlock(&xtime_lock);

update_process_times(user_mode(get_irq_regs()));
--
1.7.0.4

2010-08-10 09:39:47

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 1/4] arch/frv: Removing dead RAMKERNEL config option

Hi,

On Tue, Aug 10, 2010 at 11:11:08AM +0200, ext Christian Dietrich wrote:
>diff --git a/arch/frv/mm/init.c b/arch/frv/mm/init.c
>index ed64588..73586ad 100644
>--- a/arch/frv/mm/init.c
>+++ b/arch/frv/mm/init.c
>@@ -170,23 +170,6 @@ void __init mem_init(void)
> */
> void free_initmem(void)
> {
>-#if defined(CONFIG_RAMKERNEL) && !defined(CONFIG_PROTECT_KERNEL)
>- unsigned long start, end, addr;
>-
>- start = PAGE_ALIGN((unsigned long) &__init_begin); /* round up */
>- end = ((unsigned long) &__init_end) & PAGE_MASK; /* round down */
>-
>- /* next to check that the page we free is not a partial page */
>- for (addr = start; addr < end; addr += PAGE_SIZE) {
>- ClearPageReserved(virt_to_page(addr));
>- init_page_count(virt_to_page(addr));
>- free_page(addr);
>- totalram_pages++;
>- }
>-
>- printk("Freeing unused kernel memory: %ldKiB freed (0x%lx - 0x%lx)\n",
>- (end - start) >> 10, start, end);
>-#endif
> } /* end free_initmem() */

and now you have an empty function... how about removing the function
completely and it's uses ?

--
balbi

DefectiveByDesign.org

2010-08-10 09:42:59

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 2/4] arch/frv: Removing dead HEARTBEAT config option

Hi,

On Tue, Aug 10, 2010 at 11:11:13AM +0200, ext Christian Dietrich wrote:
>diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
>index 0ddbbae..e86df6c 100644
>--- a/arch/frv/kernel/time.c
>+++ b/arch/frv/kernel/time.c
>@@ -66,12 +66,6 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
>
> do_timer(1);
>
>-#ifdef CONFIG_HEARTBEAT

should you instead change to CONFIG_LEDS_TRIGGER_HEARTBEAT ?

--
balbi

DefectiveByDesign.org

2010-08-10 10:54:31

by Christian Dietrich

[permalink] [raw]
Subject: Re: [PATCH 1/4] arch/frv: Removing dead RAMKERNEL config option

Felipe Balbi <[email protected]> writes:

> Hi,
>
> On Tue, Aug 10, 2010 at 11:11:08AM +0200, ext Christian Dietrich wrote:
>>diff --git a/arch/frv/mm/init.c b/arch/frv/mm/init.c
>>index ed64588..73586ad 100644
>>--- a/arch/frv/mm/init.c
>>+++ b/arch/frv/mm/init.c
>>@@ -170,23 +170,6 @@ void __init mem_init(void)
>> */
>> void free_initmem(void)
>> {
>>-#if defined(CONFIG_RAMKERNEL) && !defined(CONFIG_PROTECT_KERNEL)
>>- unsigned long start, end, addr;
>>-
>>- start = PAGE_ALIGN((unsigned long) &__init_begin); /* round up */
>>- end = ((unsigned long) &__init_end) & PAGE_MASK; /* round down */
>>-
>>- /* next to check that the page we free is not a partial page */
>>- for (addr = start; addr < end; addr += PAGE_SIZE) {
>>- ClearPageReserved(virt_to_page(addr));
>>- init_page_count(virt_to_page(addr));
>>- free_page(addr);
>>- totalram_pages++;
>>- }
>>-
>>- printk("Freeing unused kernel memory: %ldKiB freed (0x%lx - 0x%lx)\n",
>>- (end - start) >> 10, start, end);
>>-#endif
>> } /* end free_initmem() */
>
> and now you have an empty function... how about removing the function
> completely and it's uses ?

If you take a look at init/main.c:826 free_initmem() is called. perhaps
this can be resolved with a macro, but if anybody in the future wants to
add some functionality there, he won't be happy after grepping.

greetz didi
--
(λ x . x x) (λ x . x x) -- See how beatiful the lambda is
No documentation is better than bad documentation
-- Das Ausdrucken dieser Mail wird urheberrechtlich verfolgt.

2010-08-10 11:06:44

by Christian Dietrich

[permalink] [raw]
Subject: Re: [PATCH 2/4] arch/frv: Removing dead HEARTBEAT config option

Felipe Balbi <[email protected]> writes:
> On Tue, Aug 10, 2010 at 11:11:13AM +0200, ext Christian Dietrich wrote:
>>diff --git a/arch/frv/kernel/time.c b/arch/frv/kernel/time.c
>>index 0ddbbae..e86df6c 100644
>>--- a/arch/frv/kernel/time.c
>>+++ b/arch/frv/kernel/time.c
>>@@ -66,12 +66,6 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
>>
>> do_timer(1);
>>
>>-#ifdef CONFIG_HEARTBEAT
>
> should you instead change to CONFIG_LEDS_TRIGGER_HEARTBEAT ?

I didn't know of this config option, but i think this would be better. I
attached a patch for it.

greetz didi
--
(λ x . x x) (λ x . x x) -- See how beatiful the lambda is
No documentation is better than bad documentation
-- Das Ausdrucken dieser Mail wird urheberrechtlich verfolgt.


Attachments:
0001-arch-frv-Rename-dead-HEARTBEAT-config-option.patch (992.00 B)

2010-08-10 11:56:22

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH 1/4] arch/frv: Removing dead RAMKERNEL config option

On Tue, Aug 10, 2010 at 12:54:16PM +0200, ext Christian Dietrich wrote:
>If you take a look at init/main.c:826 free_initmem() is called. perhaps
>this can be resolved with a macro, but if anybody in the future wants to
>add some functionality there, he won't be happy after grepping.

ok, sorry for that. Makes sense.

--
balbi

DefectiveByDesign.org