Re: segmentation fault on 1.8.21

2019-10-10 Thread Pasi Kärkkäinen
Hi,

On Sat, Aug 24, 2019 at 06:58:45AM +0200, Willy Tarreau wrote:
> On Fri, Aug 23, 2019 at 08:34:02PM -0300, Joao Morais wrote:
> > > I've now backported all the pending patches for 1.8. You can git-pull
> > > it if it's easier for you.
> > 
> > Hi Willy, current 1.8 code (commit dcb8c97) fixes the issue we found on 
> > spoe,
> > thanks!
> 
> Perfect, thanks for confirming! I'm not going to issue another 1.8 yet with
> just some minor issues though, we'll wait a bit for other fixes to accumulate.
>

..as that bug in 1.8.21 causes segfault(s) with SPOE, it would indeed be nice 
to have 1.8.22 soon..


Thanks,

-- Pasi

> Cheers,
> Willy
> 



Re: segmentation fault on 1.8.21

2019-08-23 Thread Willy Tarreau
On Fri, Aug 23, 2019 at 08:34:02PM -0300, Joao Morais wrote:
> > I've now backported all the pending patches for 1.8. You can git-pull
> > it if it's easier for you.
> 
> Hi Willy, current 1.8 code (commit dcb8c97) fixes the issue we found on spoe,
> thanks!

Perfect, thanks for confirming! I'm not going to issue another 1.8 yet with
just some minor issues though, we'll wait a bit for other fixes to accumulate.

Cheers,
Willy



Re: segmentation fault on 1.8.21

2019-08-23 Thread Joao Morais



> Em 23 de ago de 2019, à(s) 08:16, Willy Tarreau  escreveu:
> 
> On Fri, Aug 23, 2019 at 11:47:46AM +0200, Willy Tarreau wrote:
>> In the mean time you can apply the patch above. It will reject the
>> first hunk but the second one applies and will address the issue.
> 
> I've now backported all the pending patches for 1.8. You can git-pull
> it if it's easier for you.

Hi Willy, current 1.8 code (commit dcb8c97) fixes the issue we found on spoe, 
thanks!

> Thank you for this report

Thank you all for the great job on haproxy.

~jm




Re: segmentation fault on 1.8.21

2019-08-23 Thread Willy Tarreau
On Fri, Aug 23, 2019 at 11:47:46AM +0200, Willy Tarreau wrote:
> In the mean time you can apply the patch above. It will reject the
> first hunk but the second one applies and will address the issue.

I've now backported all the pending patches for 1.8. You can git-pull
it if it's easier for you.

Cheers,
Willy



Re: segmentation fault on 1.8.21

2019-08-23 Thread Willy Tarreau
Hi Joao,

On Thu, Aug 22, 2019 at 10:10:43PM -0300, Joao Morais wrote:
> 
> Hi list, I can reproduce a segmentation fault on HAProxy 1.8.21. No problem
> with 1.8.20, 1.9.10 or 2.0.5. Is there anything else I can provide or test on
> my environment?

Thank you for this report. The patch below looks highly suspicious to me :

  commit 72fdff1fdb5b82685dc3d2db23ece042195a0cbd
  Author: Christopher Faulet 
  Date:   Fri Apr 26 14:30:15 2019 +0200

MINOR: spoe: Use the sample context to pass frag_ctx info during encoding

This simplifies the API and hide the details in the sample. This way, only
string and binary are aware of these info, because other types cannot be
partially encoded.

This patch may be backported to 1.9 and 1.8.
  (...)
@@ -2187,7 +2187,9 @@ spoe_encode_message(struct stream *s, struct spoe_context 
*ctx,
 
/* Fetch the arguement value */
smp = sample_process(s->be, s->sess, s, dir|SMP_OPT_FINAL, 
arg->expr, NULL);
-   ret = spoe_encode_data(>frag_ctx.curlen, smp, 
>frag_ctx.curoff, buf, end);
+   smp->ctx.a[0] = >frag_ctx.curlen;
+   smp->ctx.a[1] = >frag_ctx.curoff;
+   ret = spoe_encode_data(smp, buf, end);
if (ret == -1 || ctx->frag_ctx.curoff)
goto too_big;
}

As you can see, it doesn't test if smp is NULL when returning from
sample_process(), and surprisingly your trace shows exactly this
function and line.

Looking at it, 2.x don't have this issue. This was apparently fixed
by this commit :

  commit 3b1d004d410129efcf365643d2583dcd2cb6ed0f
  Author: Christopher Faulet 
  Date:   Mon May 6 09:53:10 2019 +0200

BUG/MEDIUM: spoe: Be sure the sample is found before setting its context

When a sample fetch is encoded, we use its context to set info about the
fragmentation. But if the sample is not found, the function sample_process()
returns NULL. So we me be sure the sample exists before setting its context.

This patch must be backported to 1.9 and 1.8.

But despite being tagged for backport it was missed :-(

And that doesn't sound good, as apparently there was a long series of
patches merged in the same period and marked for backporting which were
forgotten, I'll have to have a look into this :-(

In the mean time you can apply the patch above. It will reject the
first hunk but the second one applies and will address the issue.

Thanks for reporting, and sorry for the messy backports, it looks like
1.8.22 will already have a bunch of patches in its queue...

Willy