[PATCH V1] um: Fix compilation warnings

[PATCH V1] um: Fix compilation warnings

[Srinivasarao Pathipati]

Use dynamic allocation in sig_handler_common() and in
timer_real_alarm_handler() to fix below warnings and build
failures where CONFIG_WERROR is enabled.

arch/um/os-Linux/signal.c: In function ‘sig_handler_common’:
arch/um/os-Linux/signal.c:51:1: error: the frame size of 2960 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
 }
 ^
arch/um/os-Linux/signal.c: In function ‘timer_real_alarm_handler’:
arch/um/os-Linux/signal.c:95:1: error: the frame size of 2960 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
 }

Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@quicinc.com>
---
 arch/um/os-Linux/signal.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/arch/um/os-Linux/signal.c b/arch/um/os-Linux/signal.c
index 24a403a..9de8826 100644
--- a/arch/um/os-Linux/signal.c
+++ b/arch/um/os-Linux/signal.c
@@ -32,23 +32,25 @@ void (*sig_info[NSIG])(int, struct siginfo *, struct uml_pt_regs *) = {
 
 static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
 {
-	struct uml_pt_regs r;
+	struct uml_pt_regs *r;
 	int save_errno = errno;
 
-	r.is_user = 0;
+	r = malloc(sizeof(struct uml_pt_regs));
+	r->is_user = 0;
 	if (sig == SIGSEGV) {
 		/* For segfaults, we want the data from the sigcontext. */
-		get_regs_from_mc(&r, mc);
-		GET_FAULTINFO_FROM_MC(r.faultinfo, mc);
+		get_regs_from_mc(r, mc);
+		GET_FAULTINFO_FROM_MC(r->faultinfo, mc);
 	}
 
 	/* enable signals if sig isn't IRQ signal */
 	if ((sig != SIGIO) && (sig != SIGWINCH))
 		unblock_signals_trace();
 
-	(*sig_info[sig])(sig, si, &r);
+	(*sig_info[sig])(sig, si, r);
 
 	errno = save_errno;
+	free(r);
 }
 
 /*
@@ -99,13 +101,15 @@ void sig_handler(int sig, struct siginfo *si, mcontext_t *mc)
 
 static void timer_real_alarm_handler(mcontext_t *mc)
 {
-	struct uml_pt_regs regs;
+	struct uml_pt_regs *regs;
 
+	regs = malloc(sizeof(struct uml_pt_regs));
 	if (mc != NULL)
-		get_regs_from_mc(&regs, mc);
+		get_regs_from_mc(regs, mc);
 	else
-		memset(&regs, 0, sizeof(regs));
-	timer_handler(SIGALRM, NULL, &regs);
+		memset(regs, 0, sizeof(struct uml_pt_regs));
+	timer_handler(SIGALRM, NULL, regs);
+	free(regs);
 }
 
 void timer_alarm_handler(int sig, struct siginfo *unused_si, mcontext_t *mc)
-- 
2.7.4


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
> static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
> {
> -	struct uml_pt_regs r;
> +	struct uml_pt_regs *r;
> 	int save_errno = errno;
> 
> -	r.is_user = 0;
> +	r = malloc(sizeof(struct uml_pt_regs));

I fear this is not correct since malloc() is not async-signal safe.

Thanks,
//richard

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Srinivasarao Pathipati]

On 2/15/2023 3:27 AM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
>> static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
>> {
>> -	struct uml_pt_regs r;
>> +	struct uml_pt_regs *r;
>> 	int save_errno = errno;
>>
>> -	r.is_user = 0;
>> +	r = malloc(sizeof(struct uml_pt_regs));
> I fear this is not correct since malloc() is not async-signal safe.

Thanks Richard for quick response. Could you please suggest alternative 
function of malloc() with async-signal safe.

if that is not possible Is there any other way to fix this warning? OR 
do we need to live with that warning?

>
> Thanks,
> //richard

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Geert Uytterhoeven]

Hi Srinivasarao,

On Wed, Feb 15, 2023 at 6:36 AM Srinivasarao Pathipati
<quic_c_spathi@quicinc.com> wrote:
> On 2/15/2023 3:27 AM, Richard Weinberger wrote:
> > ----- Ursprüngliche Mail -----
> >> Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
> >> static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
> >> {
> >> -    struct uml_pt_regs r;
> >> +    struct uml_pt_regs *r;
> >>      int save_errno = errno;
> >>
> >> -    r.is_user = 0;
> >> +    r = malloc(sizeof(struct uml_pt_regs));
> > I fear this is not correct since malloc() is not async-signal safe.
>
> Thanks Richard for quick response. Could you please suggest alternative
> function of malloc() with async-signal safe.
>
> if that is not possible Is there any other way to fix this warning? OR
> do we need to live with that warning?

Does this limit actually apply to this file, which calls into the host OS?

How come you even see this warning, as we have

    CFLAGS_signal.o += -Wframe-larger-than=4096

since commit 517f60206ee5d5f7 ("um: Increase stack frame size threshold
for signal.c") in v5.11?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Johannes Berg]

On Wed, 2023-02-15 at 09:07 +0100, Geert Uytterhoeven wrote:
> Hi Srinivasarao,
> 
> On Wed, Feb 15, 2023 at 6:36 AM Srinivasarao Pathipati
> <quic_c_spathi@quicinc.com> wrote:
> > On 2/15/2023 3:27 AM, Richard Weinberger wrote:
> > > ----- Ursprüngliche Mail -----
> > > > Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
> > > > static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
> > > > {
> > > > -    struct uml_pt_regs r;
> > > > +    struct uml_pt_regs *r;
> > > >      int save_errno = errno;
> > > > 
> > > > -    r.is_user = 0;
> > > > +    r = malloc(sizeof(struct uml_pt_regs));
> > > I fear this is not correct since malloc() is not async-signal safe.
> > 
> > Thanks Richard for quick response. Could you please suggest alternative
> > function of malloc() with async-signal safe.
> > 
> > if that is not possible Is there any other way to fix this warning? OR
> > do we need to live with that warning?
> 
> Does this limit actually apply to this file, which calls into the host OS?

Not really. Also, we know we have a signal stack that's large enough,
since we set it up ourselves:

                set_sigstack((void *) STUB_DATA, UM_KERN_PAGE_SIZE);

and it's a full page, so even the OS eating up some of that won't cause
us any trouble. We do have somewhat deep calls into do_IRQ() but those
really shouldn't use much stack space since they can (in non-UM kernels)
be called on top of arbitrary kernel stacks already.

> How come you even see this warning, as we have
> 
>     CFLAGS_signal.o += -Wframe-larger-than=4096
> 
> since commit 517f60206ee5d5f7 ("um: Increase stack frame size threshold
> for signal.c") in v5.11?
> 

Good question, I don't see it. However we probably should make that a
_bit_ smaller since we only have a page and still need to call do_IRQ()
and all.

johannes

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Johannes Berg]

On Tue, 2023-02-14 at 22:57 +0100, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
> > static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
> > {
> > -	struct uml_pt_regs r;
> > +	struct uml_pt_regs *r;
> > 	int save_errno = errno;
> > 
> > -	r.is_user = 0;
> > +	r = malloc(sizeof(struct uml_pt_regs));
> 
> I fear this is not correct since malloc() is not async-signal safe.

It would probably also be a non-trivial performance regression for
'interrupt' handling.

We _could_ use a static if this really was a problem, but that'd be just
one more thing to undo for SMP, and see my other mail, it's really not
needed to worry about htis.

johannes

_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um

Re: [PATCH V1] um: Fix compilation warnings

[Srinivasarao Pathipati]

Hi Greert Uytterhoeven,

On 2/15/2023 1:37 PM, Geert Uytterhoeven wrote:
> Hi Srinivasarao,
>
> On Wed, Feb 15, 2023 at 6:36 AM Srinivasarao Pathipati
> <quic_c_spathi@quicinc.com> wrote:
>> On 2/15/2023 3:27 AM, Richard Weinberger wrote:
>>> ----- Ursprüngliche Mail -----
>>>> Von: "Srinivasarao Pathipati" <quic_c_spathi@quicinc.com>
>>>> static void sig_handler_common(int sig, struct siginfo *si, mcontext_t *mc)
>>>> {
>>>> -    struct uml_pt_regs r;
>>>> +    struct uml_pt_regs *r;
>>>>       int save_errno = errno;
>>>>
>>>> -    r.is_user = 0;
>>>> +    r = malloc(sizeof(struct uml_pt_regs));
>>> I fear this is not correct since malloc() is not async-signal safe.
>> Thanks Richard for quick response. Could you please suggest alternative
>> function of malloc() with async-signal safe.
>>
>> if that is not possible Is there any other way to fix this warning? OR
>> do we need to live with that warning?
> Does this limit actually apply to this file, which calls into the host OS?
>
> How come you even see this warning, as we have
>
>      CFLAGS_signal.o += -Wframe-larger-than=4096
>
> since commit 517f60206ee5d5f7 ("um: Increase stack frame size threshold
> for signal.c") in v5.11?
>
> Gr{oetje,eeting}s,
>
>                          Geert

We were testing this on 5.10 kernel.

We will back port this change.

Thanks


_______________________________________________
linux-um mailing list
linux-um@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-um