[Natty] Strange beeping sound when viewing flash video

Bug #727064 reported by Id2ndR
100
This bug affects 19 people
Affects Status Importance Assigned to Milestone
GLibC
Fix Released
Medium
adobe-flashplugin (Ubuntu)
Fix Released
Undecided
Unassigned
eglibc (Ubuntu)
Fix Released
Medium
Unassigned
flashplugin-nonfree (Ubuntu)
Fix Released
Undecided
Unassigned
glibc (Fedora)
Fix Released
Medium

Bug Description

Binary package hint: adobe-flashplugin

As describe in the thread http://ubuntuforums.org/showthread.php?t=1692707 the sound is bad in natty.

Workaround: patch the flash plugin to change memcpy call to memmove
- Download https://bugzilla.redhat.com/attachment.cgi?id=460254
- Run this script: $ sudo bash ./fix-flash.sh /usr/lib/flashplugin-installer/libflashplayer.so
- Restart the browser

Revision history for this message
In , Torvalds (torvalds) wrote :
Download full text (3.1 KiB)

Created attachment 5264
Example patch to give the basic idea

I realize that it's technically "undefined", but the behavior has changed, and in the process broken existing binaries. It's a common bug to use memcpy() instead of memmove(), and the traditional behavior of "copy upwards" means that that bug can go unnoticed for a *long* time if the memory move moves things downwards.

The change was introduced in commit 6fb8cbcb58a29fff73eb2101b34caa19a7f88eba ("Improve 64bit memcpy/memmove for Atom, Core 2 and Core i7"), which was part of the 2.13 release.

As a result, upgrading from Fedora-13 to Fedora-14 caused applications like flash to fail. But it's a bug that has been reported for other applications too (and it's a bug that I've personally introduced in 'git' too - happily, people had run things like valgrind, so I don't know of any _current_ cases of it).

And there is absolutely _zero_ reason to not handle overlapping areas correctly. The "undefined behavior" is due to historical implementation issues, not because it's better than just doing the right thing.

And now applications will randomly do different things depending on the phase of the moon (technically, depending on which CPU they have and what particular version of memcpy() glibc happens to choose).

So from a pure Quality-of-Implementation standpoint, just make glibc implement memcpy() as memmove(), if you don't want to re-instate the traditional behavior that binaries have at least been tested with. Don't break random existing binaries just because the glibc version changed, and they had never been tested with the new random behavior!

I'm attaching an EXAMPLE patch against the current glibc git tree: it just tries to get rid of the unnecessary differences between memcpy and memmove for the normal ssse3 case. The approach is simple:

 - small copies (less than 80 bytes) have hand-coded optimized code that gets called through a jump table. Those cases already handle overlapping areas correctly (simply because they do all loads up-front, and stores at the end), and they are used for memmove() as-is.

   So change the memcpy() function to test for the small case first, since that's the one that can be latency critical.

 - once we're talking about bigger memcpy() cases, the extra couple of cycles to check "which direction should I copy" are totally immaterial, and having two different versions of basically the same code is just silly and is quite likely to cost more than (in I$ and page fault overhead) than just doing it in the same code. So just remove all the USE_AS_MEMMOVE games.

I haven't actually tested the patch, since building glibc is black magic and the simple "just alias __memmove_ssse3 to __memcpy_sse3" thing didn't work for me and resulted in linker errors. There's probably some simple (but subtle) magic reason for that, that I simply don't know about.

So take the patch as a "hey, doing it this way should be simpler and avoid the problem" rather than "apply this patch as-is". The ssse3-back case (which prefers backwards copies) needs similar treatment, I'll happily do that and test it if people just let me know that it's worth my time (and tell me what the black...

Read more...

Revision history for this message
In , Torvalds (torvalds) wrote :

Btw, this has hit quite a lot of people. It's Fedora

  https://bugzilla.redhat.com/show_bug.cgi?id=638477

and there are examples of having other things than just flash break (like old gstreamer plugins etc)

Revision history for this message
In , Hjl-tools (hjl-tools) wrote :

Although the current memcpy in glibc follows the spec,
some applications call memcpy with overlapping destination
and source.

I think we should help those applications without punishing
the correctly written applications. We can check an environment
variable for IFUNC, like LD_BIND_IFUNC_MEMCPY_TO_MEMMOVE.
If it is set, IFUNC memcpy will return memmove instead
of memcpy. I can prepare a patch to implement it if we
should do it.

Revision history for this message
In , Torvalds (torvalds) wrote :

So is there any real reason to believe that memmove() can't just be as fast as memcpy?

Seriously. That's what it all boils down to. Why have a separate memcpy() at all, when it clearly is correct - and nice to people - to always just implement it as a memmove(). And I really don't see the downside. It's not like it's going to be slower.

People who want to check whether their application is portable can use valgrind, the same way you have to check for portability issues in other areas (eg the extra glibc specific printf formats etc - they just "work", but they certainly aren't portable).

So why not just be nice.

If anything, from a "be nice" standpoint, it would perhaps be good to have a "debug mode", that would actually _warn_ - or even assert - about overlapping memcpy(). That obviously shouldn't be the default (since that only helps developers), but it would be along the same lines of the nice malloc debugging help that glibc can give.

IOW, I think this is an area where glibc can be _better_ than what is required of it.

Revision history for this message
In , Drepper-fsp (drepper-fsp) wrote :

The existence of code written by people who should never have been allowed to touch a keyboard cannot be allowed to prevent a correct implementation. If there is a large bod of code out there we can work around the issue for that.

We can have the existing memcpy@GLIBC_2.2.5 implementation work around the issue by avoiding the backward copying code. A new memcpy@GLIBC_2.14 could use the code currently in use.

Whether the current, new memcpy is only slightly faster than one mimicking memmove is really not that important. There are going to be more and different implementations in the future and they shouldn't be prevented from being used because of buggy programs. We should be happy to have this code here and now.

With this proposed implementation old code remains in working order until those lousy programmers use a newer platform and then they will experience the problems themselves and will fix them before releasing their code.

I'm happy to entertain a patch to this effect.

Revision history for this message
In , Jakub Jelinek (jakub-redhat) wrote :

If we go that route (symbol versioning memcpy), then wouldn't it be better to just alias the old memcpy@GLIBC_2.2.5 to memmove and have the new memcpy@@GLIBC_2.14 be the only memcpy implementation? Having two sets of memcpy implementations (especially when we have many memcpy implementations on x86_64 and i?86 selectable via STT_GNU_IFUNC), even if the workaroundish one is placed in compat .text subsections, would waste too much code section in libc.so.6.

Revision history for this message
In , Hjl-tools (hjl-tools) wrote :

(In reply to comment #5)
> If we go that route (symbol versioning memcpy), then wouldn't it be better to
> just alias the old memcpy@GLIBC_2.2.5 to memmove and have the new
> memcpy@@GLIBC_2.14 be the only memcpy implementation? Having two sets of memcpy

I like this idea.

Revision history for this message
In , Drepper-fsp (drepper-fsp) wrote :

(In reply to comment #5)
> If we go that route (symbol versioning memcpy), then wouldn't it be better to
> just alias the old memcpy@GLIBC_2.2.5 to memmove and have the new
> memcpy@@GLIBC_2.14 be the only memcpy implementation?

That's what I have in mind.

Revision history for this message
In , Felipe Contreras (felipec) wrote :

"Buggy" code can be linked to the new memcpy@@GLIBC_2.14 by just recompiling, no? That doesn't really solve anything.

Sure, code should use memcpy correctly, and if glibc has a compelling reason to break these programs, it should. As Ulrich mentions in comment #4 "There are going to be more and different implementations in the future and they shouldn't be prevented from being used because of buggy programs."

But _today_ that's not the case. Today, the regression can be fixed easily with a patch like what Linus is proposing, and there will be no *downside* whatsoever.

How about glibc breaks the behavior _only_ when there's an *upside* to breaking it?

Revision history for this message
In , Drepper-fsp (drepper-fsp) wrote :

(In reply to comment #8)
> "Buggy" code can be linked to the new memcpy@@GLIBC_2.14 by just recompiling,
> no? That doesn't really solve anything.

It solves everything. If you just relink without retesting you're an idiot and acting irresponsible.

Revision history for this message
In , Torvalds (torvalds) wrote :

(In reply to comment #9)
>
> It solves everything. If you just relink without retesting you're an idiot and
> acting irresponsible.

It does solve a lot, and at least fixes the "you broke stuff that used to work" issue.

However, I still don't understand why you guys can't just admit that you might as well just do memmove() and be done with it. It's not slower. And the excuse that "you'll have more implementations in the future" is just an even stronger reason to do that.

To make this very clear: your new "and improved" memcpy() ACTS DIFFERENTLY ON DIFFERENT MACHINES. That means that all that testing that was done by the developer at link-time is ALMOST TOTALLY WORTHLESS, because what was tested wasn't necessarily at all what the user sees.

I really don't understand why you can't admit that random behavior like that by a very fundamental core library routine is actually a real problem.

And there really isn't any upside. The optimized routines up to 80 bytes are the same (in fact, my patch should speed them up by a few cycles), and anything bigger than that will not notice the extra couple of cycles to check for overlap.

So while I agree that it's a fix to the immediate problem to just say "don't screw up for existing binaries", I do NOT understand why the glibc people then apparently think it's fine to be stupid for new binaries.

Revision history for this message
In , Felipe Contreras (felipec) wrote :

(In reply to comment #9)
> (In reply to comment #8)
> > "Buggy" code can be linked to the new memcpy@@GLIBC_2.14 by just recompiling,
> > no? That doesn't really solve anything.
>
> It solves everything. If you just relink without retesting you're an idiot and
> acting irresponsible.

You cannot test every possible code-path.

Revision history for this message
In , Oliver-henshaw (oliver-henshaw) wrote :

The thread starting at http://lwn.net/Articles/414496/ may be instructive on how this can cause problems despite the good(-ish) intentions of the developer. In short, squashfs used memcpy with data that was known not to overlap but later changes invalidated this assumption.

Revision history for this message
In , Bvasselle (bvasselle) wrote :

(In reply to comment #9)
> (In reply to comment #8)
> > "Buggy" code can be linked to the new memcpy@@GLIBC_2.14 by just recompiling,
> > no? That doesn't really solve anything.
>
> It solves everything. If you just relink without retesting you're an idiot and
> acting irresponsible.

Nobody is interested in an optimization in lib C that would at best gain less than the simple fact of calling the function.

Everybody is interested in using the code that idiots may produce.

Revision history for this message
In , Hjl-tools (hjl-tools) wrote :

Created attachment 5323
A patch

This works for me.

Revision history for this message
In , Jakub Jelinek (jakub-redhat) wrote :

This is not correct:
1) glibc 2.13 has been released already, so new symbol versions must be GLIBC_2.14
2) you do it only on x86_64, therefore you should add it into sysdeps/x86_64/Versions (though, you will need to add GLIBC_2.14 to toplevel Versions.def too)

272 comments hidden view all 338 comments
Revision history for this message
In , torvalds (torvalds-redhat-bugs) wrote :

(In reply to comment #253)
>
> Kindly read comment #99.

Bullshit. Read comment #132. Even if you want to copy backwards, there is absolutely zero reason to not check the overlapping case first, to see that you don't do it wrong.

Why is that so hard for people to understand?

The _only_ reason to do memcpy() that clobbers overlapping areas is that the code is fast BECAUSE IT IS SIMPLE. So if you were to use "rep movsb" as the memcpy implementation, then you have a good argument why it does the overlapping case wrong - it's so simple as to be brainless.

But once you do what the glibc memcpy does, there is just no excuse any more for it.

Revision history for this message
In , ricky (ricky-redhat-bugs-1) wrote :

Precisely, it's a trade-off, not an improvement. The argument for keeping it is as good as saying: "Let's make it do nothing at all - at least it'll be faster!".

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :
Download full text (3.8 KiB)

(In reply to comment #253)
> (In reply to comment #252)
> > (In reply to comment #248)
> > > If you don't want the burden of choosing correctness over speed, don't program
> > > in C.
> >
> > Have you measured the speed difference between memcpy and memmove? No? Then
> > don't assert that one is faster than the other, because they aren't. This has
> > been demonstrated in the thread with numbers.
>
> Kindly read comment #99.

Hopefully Linus has answered this one.

> > > Does anyone read the thread? Flash works fine, its only people insisting on
> > > using an unsupported (by Adobe themselves) 64-bit preview version of flash
> > > having a problem.
> >
> > AFAIK it happens on 32-bit too,
>
> Read the thread. No one has said any such thing. If you have experienced
> otherwise, please let us know.

I did read the thread, everybody mentioned 64-bit instead of "the square version", but Pablo Rodríguez mentioned he experiences the issue in 32-bit too. Anyway, I tried on my 32-bit laptop, and it doesn't seem to run, so I guess that's a different issue.

> > it's only a matter of time before the "square"
> > code gets into the "non-preview" version of Flash.
>
> A hypothetical statement that makes a lot of assumptions about Adobe's
> development process. Do you work for Adobe?
>
> They could also have committed a fix into their source tree months ago and its
> just a matter of time until they release a fixed version of the 64-bit plugin.
>
> I don't work for Adobe and I don't see you with an @adobe.com email so my
> hypothetical statements are just as good as yours.

The 'square' plug-in is 10.3, and the current one is 10.2. If you don't put code in 10.3 beta that you would use in 10.3 final, what's the point of the beta then? Sure, maybe they fixed that internally, but maybe not.

> > Anyway, this bug is _not_ about Flash, it's about glibc. Anything that uses
> > memcpy can be affected silently. How much code is misbehaving thanks to this
> > change? It's impossible to know.
>
> If a tree falls in the woods...
>
> > For more examples of issues, see:
> > http://lwn.net/Articles/414496/
>
> Just read it, squashfs fixed their code, problem solved.

No, problem not solved. That's _one_ issue. Can you be certain that there are no issues lingering there? How many millions of line of code can be affected by this? Sure, we've found so far a few issues that were obvious, but there could be many more hidden, specially if they are subtle, or tricky to trigger.

> It also links to this:
>
> http://article.gmane.org/gmane.comp.lib.glibc.alpha/15278
>
> "This patch includes optimized 64bit memcpy/memmove for Atom, Core 2 and
> Core i7. It improves memcpy by up to 3X on Atom, up to 4X on Core 2 and
> up to 1X on Core i7. It also improves memmove by up to 3X on Atom, up to
> 4X on Core 2 and up to 2X on Core i7."

So? memcpy can be improved, and so can memmove. But they are the same. So again, this potential breakage provides _zero_ gain.

> > So, not fixing this bug means silently breaking stuff with _zero_ gain.
>
> Your "zero gain" premise is contradicted by at least two Intel engineers.
>
> And Linus never said what hardware he was performance ...

Read more...

273 comments hidden view all 338 comments
Revision history for this message
In , Felipe Contreras (felipec) wrote :

So we have two solutions:

1) The solution proposed by Linus Torvalds, attachment #5264

Advantages: everything works the same as before
Disadvantages: a few extra cycles in certain memcpy's

2) The solution proposed by Ulrich Drepper, attachment #5323

Advantages: old binaries keep working
Disadvantages: newly compiled code remains affected

Clearly 1) is the most sensible solution as the only advantage of 2) is a few cycles, and has drastic disadvantages.

25 comments hidden view all 338 comments
Revision history for this message
Andreas Oberritter (mtdcr) wrote :

I've added eglibc, because the regression is caused by an upstream patch to glibc, which changed the semantics of memcpy. This patch is likely to have caused subtle problems in other packages than adobe-flashplugin, too.

I wonder why this bug gained so much attention at Fedora but didn't receive any comments here. Is this a duplicate of another bug report I couldn't find?

Please either revert the upstream patch or alias memcpy to memmove as suggested in https://bugzilla.redhat.com/show_bug.cgi?id=638477 before releasing 11.04.

Revision history for this message
Andreas Oberritter (mtdcr) wrote :

Adobe Flash Player bug tracker URL: https://bugs.adobe.com/jira/browse/FP-5739

Revision history for this message
Andreas Oberritter (mtdcr) wrote :
298 comments hidden view all 338 comments
Revision history for this message
In , siarhei.siamashka (siarhei.siamashka-redhat-bugs) wrote :

(In reply to comment #254)
> (In reply to comment #253)
> >
> > Kindly read comment #99.
>
> Bullshit. Read comment #132. Even if you want to copy backwards, there is
> absolutely zero reason to not check the overlapping case first, to see that you
> don't do it wrong.

I love how you use the words 'absolutely' and 'zero reason' :) Ever heard about http://en.wikipedia.org/wiki/Cargo_cult_programming ?

> Why is that so hard for people to understand?
>
> The _only_ reason to do memcpy() that clobbers overlapping areas is that the
> code is fast BECAUSE IT IS SIMPLE. So if you were to use "rep movsb" as the
> memcpy implementation, then you have a good argument why it does the
> overlapping case wrong - it's so simple as to be brainless.
>
> But once you do what the glibc memcpy does, there is just no excuse any more
> for it.

That's just pure bullshit. Making assumptions about how exactly undefined behavior might manifest itself with various C library implementations is not something that you should rely on when developing software. What is so hard to understand here?

GNU/Linux is not the only OS which tries to be POSIX compatible, glibc is not the only C library and gcc is not the only C compiler. What you and your fanboys are doing here is just an aggressive promotion of a trivial memcpy misuse bug into a new de-facto standard. I would be really upset if "it is safe because glibc is known to work this way" becomes a common practice and a valid excuse for not fixing bugs.

There are many bugs being introduced and fixed in various software daily. What makes this particular trivial bug so special that it even deserves an update for the C language standard? Especially considering that there are multiple tools capable of detecting this overlapping memcpy problem and it is almost nonexistent in practice.

This bug also highlights a major weakness of the Flash plugin. For the various security problems not addressed over long periods of time they might have an excuse, maybe the bugs were not so easy to fix. But based on how this trivial memcpy issue is being handled, looks like Adobe just does not have a sane process for releasing updates and security fixes. This is very disturbing.

Revision history for this message
In , awilliam (awilliam-redhat-bugs) wrote :

Once again: this bug is not the right place to discuss this. Fedora is not where significant changes to glibc behaviour happen. Fedora Bugzilla is not the right place to discuss making them. There is already an upstream bug for this. Fedora will follow the approach that is decided on upstream, when it is decided on upstream. Commenting here is achieving precisely nothing.

Revision history for this message
In , davidsen (davidsen-redhat-bugs) wrote :

(In reply to comment #257)

> There are many bugs being introduced and fixed in various software daily. What
> makes this particular trivial bug so special that it even deserves an update
> for the C language standard? Especially considering that there are multiple
> tools capable of detecting this overlapping memcpy problem and it is almost
> nonexistent in practice.
>
Why are developers fighting this so hard? If upstream introduces new code to the kernel which breaks existing programs, it gets fixed in Fedora. Here old versions of the library work with existing code and not with the update. Why is good to fix kernel bugs and bad to fix library bugs.

I'm sure RHEL will not introduce a change which breaks existing programs, why should Fedora? Put the "standard conforming" library in FC15 and be happy, hopefully it will break GNOME3. But unless the Fedora team intends to rewrite and maintain all of the other Fedora software which is using the wrong move, the place to fix the disfunction is at the library, and not deliberately break programs in the names of pedantic adherence to a standard.

> This bug also highlights a major weakness of the Flash plugin. For the various
> security problems not addressed over long periods of time they might have an
> excuse, maybe the bugs were not so easy to fix. But based on how this trivial
> memcpy issue is being handled, looks like Adobe just does not have a sane
> process for releasing updates and security fixes. This is very disturbing.

I'm less disturbed by slow support from Adobe than calling a bug a feature by Fedora.

Revision history for this message
In , jakub (jakub-redhat-bugs-1) wrote :

Perhaps because it is clearly documented?

ISO C99, 7.21.2.1:
"If copying takes place between objects that overlap, the behavior is undefined."
Ditto ISO C90, 7.11.2.1, Unix 98, POSIX 2003, POSIX 2008.
man 3 memcpy also clearly says
"The memory areas should not overlap. Use memmove(3) if the memory areas do overlap."
After all that is the only difference between memcpy and memmove.

glibc certainly doesn't guarantee bug compatibility, backward compatibility is only provided for correctly written programs that don't trigger undefined behavior. I'm not sure why you are rising RHEL here, the upcoming major version of RHEL will certainly provide the upstream memcpy version.

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

(In reply to comment #260)
> Perhaps because it is clearly documented?

So? The purpose of Fedora is not to follow some specification documentation, it is to provide stable, reliable, useful system.

If breaking applications was done in favor of some drastic performance improvements, that might be ok, following API break path that upstream is planning to do.

But there's nothing like that. You are fighting to protect a vacuum. You want to break applications in order to gain _nothing_.

I guess it's more important to make a few developers at RedHat feel good about themselves because they manage to close a bug without moving a finger, and claiming POSIX correctness, than to make the system more reliable.

Congratulations, you have knowingly made the system more unreliable in order to gain nothing.

277 comments hidden view all 338 comments
Revision history for this message
In , Felipe Contreras (felipec) wrote :

Actually, why not have both? I think this plan would fit everyone:

1) Apply Linus' patch

Both to 2.13 and 2.14, this would not introduce any issues and would restore back the previous behavior, so applications would still work. As I mentioned before, the disadvantages are minimal.

2) Apply H.J. Lu's patch

This would go to 2.14, but not only to x86 but all architectures, and add an overlap check, and when triggered issue a crash.

This would allow old binaries to keep working on 2.14, but newly compiled ones would crash if they are doing something wrong. This would allow all the users of glibc to fix their code for the impending changes.

3) Remove overlap checks

On 2.15, after a transition period, the overlap checks can be removed, and thus save the few extra cycles.

This has all the advantages of all the proposals, and makes it easy to fix the applications that are using memcpy wrong.

Revision history for this message
In , Bvasselle (bvasselle) wrote :

(In reply to comment #17)
> Actually, why not have both? I think this plan would fit everyone:

No, it does not. It certainly does not.

It is not only the problem of recompiling the existing code, it's also the problem of fixing it and re-qualifying it. This plan has a huge cost... and it's vain.

The contract C programmers have with C and the C library is clear: we accept a fair amount of inefficency, but we don't have to program in assembly nor care about the system's internals. How many people still use the C library when it comes to be important to gain an addition plus a comparison?

The problem we're facing just made this fact plain: there is no reason why memcpy should not be memmove.

Revision history for this message
In , Felipe Contreras (felipec) wrote :

(In reply to comment #18)
> (In reply to comment #17)
> > Actually, why not have both? I think this plan would fit everyone:
>
> No, it does not. It certainly does not.
>
> It is not only the problem of recompiling the existing code, it's also the
> problem of fixing it and re-qualifying it. This plan has a huge cost... and
> it's vain.

I understand that, but it's better than the current alternative that Ulrich is more likely to merge, which is basically to do nothing.

Revision history for this message
In , Felipe Contreras (felipec) wrote :

Created attachment 5341
Patch to check for overlaps

I'm trying this patch to find out how many things in the system use overlapping memcpy, however, it doesn't seem to cause any crashes... Can anyone spot something wrong?

275 comments hidden view all 338 comments
Revision history for this message
In , shigorin (shigorin-redhat-bugs) wrote :

(In reply to comment #253)
> Why is that so hard for people to understand?
Maybe because they failed to grok Postel's Law?

In the meanwhile, folks thank you for the master class and point fingers at fedora developers: http://avva.livejournal.com/2323823.html (in Russian)

2 jj: je svedomi? :( there are too few _correctly_ written programs in the world

Revision history for this message
In , linuxover (linuxover-redhat-bugs) wrote :

(In reply to comment #101)
> I'd personally suggest that glibc just alias memcpy() to memmove().

I think You aren't right :)

You suggest to provide *one* of many undocumented behaviours. What do You think do with other undocumented behaviours?

for example: memcpy can (or could) be used to propagate a pattern:

strcpy(buffer, "abc");
memcpy(buffer + 3, buffer, 100);

it will (or would) fill buffer by repeating "abcabcabc..."

Someone can invent the other undocumented variant. If You try provide all these variants You will have no time to do something in linux kernel.

I think that invalid (adobe's) code must be fixed anyway. memcpy shouldn't be alias to memmove.

275 comments hidden view all 338 comments
Revision history for this message
In , Nick Shaforostoff (shafff) wrote :

i'd like to vote for having memcpy and memmove identical.

(unless there are benchmarks that are showing that perf difference is high)

276 comments hidden view all 338 comments
Revision history for this message
In , linuxover (linuxover-redhat-bugs) wrote :

> I'm no great fan of flash but it's an essential part of life on the web these
> days and I had thought that the Fedora project had finally put its days of
> broken flash support behind it.

to watch Youtube I use gnash. Last version works well.

Revision history for this message
In , milan.kerslager (milan.kerslager-redhat-bugs) wrote :

A programmer should not use side effects of any function.
A glibc should not provide function with performance hit for whatever reason.
It does not matter in what situation has been regression observed.
Glibc has performance regression. This should be fixed.
This has been observed by (broken) Flash (but this hits not only Flash).
Flash had broken code before that. This has been observed by glibc regression.
So Flash should to be fixed too.
Broken design of a function(s) should not leads to broken implementation.
Etc, etc...

Revision history for this message
In , knutjbj (knutjbj-redhat-bugs) wrote :

It is also present in Neverwinter nights.

277 comments hidden view all 338 comments
Revision history for this message
In , Drepper-fsp (drepper-fsp) wrote :

HJ's patch which implements what I proposed is in git.

Revision history for this message
In , Hjl-tools (hjl-tools) wrote :

(In reply to comment #3)
>
> If anything, from a "be nice" standpoint, it would perhaps be good to have a
> "debug mode", that would actually _warn_ - or even assert - about overlapping
> memcpy(). That obviously shouldn't be the default (since that only helps

You can use LD_AUDIT to do it today.

29 comments hidden view all 338 comments
Revision history for this message
Colin Watson (cjwatson) wrote :

This has been fixed (at least to avoid breaking old binaries) here:

  http://sourceware.org/git/?p=glibc.git;a=commitdiff;h=0354e355014b7bfda32622e0255399d859862fcd

I wonder what the right way to backport this is, though. Matthias, do you think it's OK to end up with a GLIBC_2_14 symbol? We'd have to be rather careful about the .symbols files until such time as we have a real 2.14 release.

Changed in eglibc (Ubuntu):
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Matthias Klose (doko) wrote :

A test build is currently building in the ubuntu-toolchain-r archive. Please check the build once it is completed. The mem* multiarch functions are disabled in this build. It's a temporary change, and will be reverted with glibc-2.14.

see https://launchpad.net/~ubuntu-toolchain-r/+archive/ppa/

add to /etc/apt/sources.list:

deb http://ppa.launchpad.net/ubuntu-toolchain-r/ppa/ubuntu natty main
deb-src http://ppa.launchpad.net/ubuntu-toolchain-r/ppa/ubuntu natty main

Changed in eglibc (Ubuntu):
status: Triaged → In Progress
Revision history for this message
Markus Birth (mbirth) wrote :

There's a patch for the current flashplayer64: http://www.linux.org.ru/forum/talks/5663681 . Works fine for me. Also while parsing through Google results, I found that this only affects MP3 sound, not AAC. YouTube's 240px videos (and some very old ones) use MP3 sound, all higher resolutions use AAC. Just FYI.

Revision history for this message
Matthias Klose (doko) wrote :

Markus, please could you recheck without the "patch", but with the proposed eglibc version?

Revision history for this message
Jacob Peddicord (jpeddicord) wrote :

I gave it (the rebuilt eglibc) a go, and it appears to solve the issue. Haven't noticed any side-effects, either.

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package eglibc - 2.13-0ubuntu12

---------------
eglibc (2.13-0ubuntu12) natty; urgency=low

  * For memcpy-ssse3, enable chk symbols in static builds. LP: #726802.
  * Disable the memcpy multiarch implementaiton on x86_64. LP: #727064.
  * Merge from Debian:
    - Add patches/i386/cvs-cacheinfo.diff to fix empty LEVEL*CACHE* getconf()
      entries for some CPU. Closes: #609389.
 -- Matthias Klose <email address hidden> Tue, 05 Apr 2011 10:54:32 +0200

Changed in eglibc (Ubuntu):
status: In Progress → Fix Released
302 comments hidden view all 338 comments
Revision history for this message
In , brendan.jones.it (brendan.jones.it-redhat-bugs) wrote :

*** Bug 680802 has been marked as a duplicate of this bug. ***

Revision history for this message
In , riku.seppala (riku.seppala-redhat-bugs) wrote :
Revision history for this message
In , awilliam (awilliam-redhat-bugs) wrote :

so, that patch should get pulled downstream now.

278 comments hidden view all 338 comments
Revision history for this message
In , Bvasselle (bvasselle) wrote :

(In reply to comment #4)
> The existence of code written by people who should never have been allowed to
> touch a keyboard cannot be allowed to prevent a correct implementation. If
> there is a large bod of code out there we can work around the issue for that.
>
Don't you see it is just NOT a correct implementation?

Revision history for this message
In , Felipe Contreras (felipec) wrote :

Created attachment 5660
Patch to check for overlaps

Actually the code from Linus had a bug in the 'cmp' checks, here's the updated version.

I just started to run my Fedora system with this, and I already see crashes in pulseaudio and readahead-collector. I will continue running this for a bit, but I think it's pretty clear that we should not assume that all applications in a typical system have been using memcpy properly.

So, again, I think we need at least a transition period so that people can detect and fix the issues.

278 comments hidden view all 338 comments
Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

Created attachment 491126
x86_64: workaround for new memcpy behavior

I have backported the fix to 2.13, and I've launched a koji build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=2992294

Enjoy :)

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

Created attachment 491127
memcpy-ssse3: add overlap checks

I used Linus' patch as a guide to add overlap checks that would crash improper apps, and tried to boot my Fedora 14 system with it.

I noticed pulseaudio and readahead-collector crash, so they must be doing something wrong.

I say if Fedora cares about the quality of the full system something like this should be done to find all the bad code.

Revision history for this message
In , jc1da.3011 (jc1da.3011-redhat-bugs) wrote :

I'm testing testing Fedora 15 ... Problem is fixed now

Revision history for this message
In , jakub (jakub-redhat-bugs-1) wrote :

(In reply to comment #270)
> Created attachment 491126 [details]
> x86_64: workaround for new memcpy behavior
>
> I have backported the fix to 2.13, and I've launched a koji build:
>
> http://koji.fedoraproject.org/koji/taskinfo?taskID=2992294

This is ABI incompatible library, so please don't spread this out.

Revision history for this message
In , brentrbrian (brentrbrian-redhat-bugs) wrote :

Will this be made available as an yum update on FC14 ?

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

Created attachment 491198
x86_64: fix for new memcpy behavior (try 2)

Here's a second try, should achieve the same thing, but ABI compatible.

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

(In reply to comment #275)
> Created attachment 491198 [details]
> x86_64: fix for new memcpy behavior (try 2)
>
> Here's a second try, should achieve the same thing, but ABI compatible.

Here's the corresponding koji build, now it's ABI compatible.

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :
Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

Thanks to Ulrich Drepper I managed to write a simple check to find memcpy overlaps issues system-wide and I created a tracking but to link my findings so far. It would be useful if other people ran this on their systems as well:

Bug #696096.

285 comments hidden view all 338 comments
Revision history for this message
In , Vincent+libc (vincent+libc) wrote :

(In reply to comment #18)
> The problem we're facing just made this fact plain: there is no reason why
> memcpy should not be memmove.

If these two functions should behave in the same way, then why not all programmers use memmove (which has fewer requirements)?

If memcpy is called while the objects overlap, the bug is not necessarily that memmove should have been used instead. The cause may be an incorrect size. So, with this point of view, it should be safer to abort than letting the program behave in an uncontrolled way.

(In reply to comment #25)
> So, again, I think we need at least a transition period so that people can
> detect and fix the issues.

But it may be difficult to detect the issues. For instance, zsh was affected by a similar problem (now fixed in CVS only) with the optimized strcpy, but to detect the problem, it involves keyboard input:

  http://www.zsh.org/mla/workers/2011/msg00533.html
  http://www.zsh.org/mla/workers/2011/msg00544.html

For strcpy, this is even worse, as there is no strmove function, so that either programmers have to write non-portable code or they have to reimplement a naive version of strcpy or find some other workaround, such as memmove + strlen:

  http://www.zsh.org/mla/workers/2011/msg00542.html

I suppose that if this has been done for memcpy, then strcpy should also be patched in some way...

Revision history for this message
In , Felipe Contreras (felipec) wrote :

(In reply to comment #26)
> (In reply to comment #25)
> > So, again, I think we need at least a transition period so that people can
> > detect and fix the issues.
>
> But it may be difficult to detect the issues. For instance, zsh was affected by
> a similar problem (now fixed in CVS only) with the optimized strcpy, but to
> detect the problem, it involves keyboard input:

Yes, it is difficult, that's why I think it should be enabled globally.

BTW. Here's another issue on zsh I found while enabling the memcpy checks:
https://bugzilla.redhat.com/show_bug.cgi?id=698439

285 comments hidden view all 338 comments
Revision history for this message
In , simon.lewis (simon.lewis-redhat-bugs) wrote :

Ubuntu is also using the MEMCPY hack - the deb package refers to redhat bug #638477

See http://www.ubuntuupdates.org/packages/show/301005

I guess Ubuntu is wating for redhat to fix glibc.......

Revision history for this message
In , bill-bugzilla.redhat.com (bill-bugzilla.redhat.com-redhat-bugs) wrote :

Adobe didn't fix this in Flash 10.3. :( I applied Ray Strode's script to the 10.3 binary and it still appears to work.

2.6.35.13-91.fc14.x86_64
glibc-2.13-1.x86_64

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

(In reply to comment #280)
> Adobe didn't fix this in Flash 10.3. :( I applied Ray Strode's script to the
> 10.3 binary and it still appears to work.

Flash 10.3 is ambiguous, 10.3.162 is flash "square" preview 3 for 64-bit systems, 10.3.181.14 is 10.3 final, for 32-bit systems.

315 comments hidden view all 338 comments
Revision history for this message
Baptiste Coudurier (baptiste-coudurier) wrote :

May I ask why has the fix been reverted in oneiric ?

Changed in glibc:
importance: Unknown → Medium
status: Unknown → Fix Released
316 comments hidden view all 338 comments
Revision history for this message
In , m.heiming (m.heiming-redhat-bugs) wrote :

Thx a bunch for Linus quick patch, saves me from the annoying sound with youtube videos on FC 14...

Revision history for this message
In , brentrbrian (brentrbrian-redhat-bugs) wrote :

Patch works for me as well

Revision history for this message
In , mike (mike-redhat-bugs-1) wrote :

Andreas or Jakub, what is the status of this?

I just ran into an overlapping memcpy() in a piece of third-party proprietary software used by my $DAYJOB. It seems this memcpy() change is in RHEL6 as the customers effected are running RHEL6. I can, of course, fix the problem, but other softwares out there may be effected, too. RHEL6 may need to be fixed as other proprietary houses may not be so friendly.

Revision history for this message
In , mike (mike-redhat-bugs-1) wrote :

glibc-2.14-3 still exhibits the new, backwards behavior for me. (Fedora 15)

Is this expected?

Revision history for this message
In , mjg (mjg-redhat-bugs) wrote :

For anything built against new glibc, yes. Old binaries should pick up the old memcpy behaviour.

Revision history for this message
In , felipe.contreras (felipe.contreras-redhat-bugs) wrote :

FTR the public beta of Flash 11 seems to have the problem fixed.

Revision history for this message
In , sebastien.major (sebastien.major-redhat-bugs) wrote :

Does the subject can be a concrete one ?
I did not read as carefully as evidently needed but, as entirely user under Linux and after GNU/Linux since 1994, I think we could just get away from this talk and move the debate target.

Super : adobe done the new glibc update.

In my humble opinion, we need all people to raise up Linux and not read comments that disturb the project. If Linus Torvalds said ... We must follow, unless, it's stupid and get us or another in danger ! Completely agree with the facts of he told.

What about have memcpy be exact aliasing to memset and memcpy32, memcpy64, memcpy128, ... be other functions ?

Please, don't bicker yourselves about this one year subject Bug. I think we need you for largely others things.

Revision history for this message
In , kolyshkin (kolyshkin-redhat-bugs) wrote :

Guys,

Thanks for the solution! I was tired of manually adding LD_PRELOAD to firefox/google-chrome start script, so I ended up automating the process using RPM triggers. Now all I (and you) need to do is to install flash-fix RPM and forget about the problem.

Get the stuff from here: http://kir.sacred.ru/flash-fix/

Revision history for this message
In , riku.seppala (riku.seppala-redhat-bugs) wrote :

Adobe has updated flash plugin that fix this. Works for me.

http://labs.adobe.com/downloads/flashplayer11.html

Revision history for this message
In , noloader (noloader-redhat-bugs) wrote :

(In reply to comment #152)
> ...
> I would like to urge anyone with any political power withing Fedora to get this
> problem dealt with immediately. An organization this big and mature shouldn't
> be having trouble dealing with this sort of thing, fix it, make certain it
> won't happen again and move on.
Unfortunately, Fedora politcos will probably not be able to get Adobe to move any faster or do a better job with their software. Adobe has a chronic, progressive problems (which, not surprisingly, spills over into security related defects). There's a reason Apple selectively bans Adobe from their platforms.

"Adobe surpasses Microsoft as favorite hacker’s target" [1]
"Adobe predicted as top 2010 hacker target" [2]
"Adobe products are legendary for their insecurity." [3]

Adobe security related history: http://www.google.com/#q=adobe+pointer+site:securityfocus.com

[1] http://lastwatchdog.com/adobe-surpasses-microsoft-favorite-hackers-target/
[2] http://www.theregister.co.uk/2009/12/29/security_predictions_2010/
[3] http://techcrunch.com/2011/08/20/revenge-of-the-killer-script-kiddies/

Revision history for this message
In , vik (vik-redhat-bugs) wrote :

Garbled audio on Fedora 14 with flash also happens when using:
http://demo.bigbluebutton.org/

The workaround in this case has no effect.

297 comments hidden view all 338 comments
Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in adobe-flashplugin (Ubuntu):
status: New → Confirmed
Changed in flashplugin-nonfree (Ubuntu):
status: New → Confirmed
Naël (nathanael-naeri)
Changed in adobe-flashplugin (Ubuntu):
status: Confirmed → Fix Released
Changed in flashplugin-nonfree (Ubuntu):
status: Confirmed → Fix Released
298 comments hidden view all 338 comments
Revision history for this message
In , davidjoelschwartz (davidjoelschwartz-redhat-bugs) wrote :

"Congratulations, you have knowingly made the system more unreliable in order to gain nothing."

Actually, it's not nothing.

Several bugs were discovered as a result of this change and are now being fixed. This seems to be a somewhat common bug and if it has no consequences, it will continue to become more and more widespread until the day it does have consequences. The sooner that day is, the less it will break.

In addition, in the future if modifications to memcpy do make significant optimizations possible, they won't break things. One of the obstacles to innovation is dealing with code that abuses undocumented behavior and that can lead to a culture of having to maintain bug-for-bug compatibility.

We are talking about a function that exists for exactly one purpose and it being made unsuitable for that purpose.

Changed in glibc (Fedora):
importance: Unknown → Medium
status: Unknown → Fix Released
Displaying first 40 and last 40 comments. View all 338 comments or add a comment.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.