skas3 patch breaks ATI video drivers

uml-devel Marcus Better 3 participants 10 messages Jan 24, 2005 – Jan 28, 2005 1 message not in the archive

Subsystems (2)

User-mode Linux (UML) ptrace support

lore

Share & tools

lore

Marcus Better

author

2005-01-24 19:44 UTC #0 lore raw

Hi all,

I applied Blaisorblade's host-skas3-2.6.9-v7.patch, and it breaks the fglrx graphics driver from ATI[1]. This is with Debian kernel 2.6.9, but I believe it applies to vanilla kernels also.

The problem is the following declaration added to include/asm-i386/desc.h:

----------------------------------------------------------
extern int modify_ldt(struct mm_struct * mm, int func, void __user *ptr,

		      unsigned long bytecount);

----------------------------------------------------------

The fglrx driver contains the following macro in a source file:

_syscall3( int, modify_ldt, int, func, void *, ptr, unsigned long, bytecount )

Here the compiler chokes with the following error:
---------------------------------------------------------
/usr/src/modules/fglrx-4.3.0-3.14.6/firegl_public.c:181: error: conflicting types for modify_ldt' include/asm/desc.h:144: error: previous declaration of modify_ldt'
---------------------------------------------------------

I don't understand the details of the kernel code, but it seems that the problem is that the SKAS patch defines the function modify_ldt which conflicts with the syscall macros.

I renamed the new function to _modify_ldt instead (and changed the two or three references to it in ldt.c), and this seems to fix the problem.

Please CC replies to me, I'm not subscribed to the list.

Thanks for your work!

Marcus B.

[1] http://www.ati.com/support/drivers/linux/radeon-linux.html

-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting Tool for open source databases. Create drag-&-drop reports. Save time by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc. Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Blaisorblade

2005-01-24 19:55 UTC #1 lore raw

Quoted 25 lines · Marcus Better

Hi all,
I applied Blaisorblade's host-skas3-2.6.9-v7.patch, and it breaks the fglrx graphics driver from ATI[1]. This is with Debian kernel 2.6.9, but I believe it applies to vanilla kernels also.
The problem is the following declaration added to include/asm-i386/desc.h:
----------------------------------------------------------
extern int modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
        unsigned long bytecount);
----------------------------------------------------------
The fglrx driver contains the following macro in a source file:
_syscall3( int, modify_ldt, int, func, void *, ptr, unsigned long, bytecount )
Here the compiler chokes with the following error:
---------------------------------------------------------
/usr/src/modules/fglrx-4.3.0-3.14.6/firegl_public.c:181: error:
conflicting types for modify_ldt' include/asm/desc.h:144: error: previous declaration of modify_ldt'
---------------------------------------------------------

Quoted 6 lines

I don't understand the details of the kernel code, but it seems that the problem is that the SKAS patch defines the function modify_ldt which conflicts with the syscall macros.
I renamed the new function to _modify_ldt instead (and changed the two or three references to it in ldt.c), and this seems to fix the problem.

I.e. the compilation problem is not in the kernel module, right? In this case, the problem is that you have /usr/include/asm symlinked to your kernel's include/asm-i386 directory, which is the wrong setup. However your solution should not give any problem.

If this is wrong, instead, then you are surely right... but then the solution you explain would be unlikely to fix anything... and the code would also be different, I guess. It's just a quick shoot however, so feel free to correct me.

Quoted 3 lines

Please CC replies to me, I'm not subscribed to the list.
Thanks for your work!

Thanks to you for using UML!

Quoted 3 lines

Marcus B.
[1] http://www.ati.com/support/drivers/linux/radeon-linux.html

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Marcus Better

author

2005-01-25 08:35 UTC #2 lore raw

Quoted 1 line · Blaisorblade

I.e. the compilation problem is not in the kernel module, right?

Yes, the compilation error is in compiling the fglrx module.

Quoted 1 line

In this case, the problem is that you have /usr/include/asm symlinked to your kernel's include/asm-i386 directory,

No, I don't, but I use Debian's make-kpkg which probably uses header files from the kernel tree. This is the normal method of building kernels on Debian, though.

I might add that I tried to fix the problem directly in the fglrx module, but it doesn't seem to work. It really must use the name modify_ldt in the macro, and this conflicts with the UML patch.

Marcus

Blaisorblade

2005-01-25 09:44 UTC #3 lore raw

Quoted 4 lines · Marcus Better

Quoted 1 line · Blaisorblade
I.e. the compilation problem is not in the kernel module, right?
Yes, the compilation error is in compiling the fglrx module.

Ok, unexpected.

Quoted 6 lines

Quoted 2 lines
In this case, the problem is that you have /usr/include/asm symlinked to your kernel's include/asm-i386 directory,
No, I don't, but I use Debian's make-kpkg which probably uses header files from the kernel tree. This is the normal method of building kernels on Debian, though.

Well, since it is a kernel module, then it's correct...

Quoted 5 lines

I might add that I tried to fix the problem directly in the fglrx module, but it doesn't seem to work.
It really must use the name
modify_ldt in the macro

Yes, because the name in the macro is used to build the "__NR_modify_ldt"

Quoted 1 line

, and this conflicts with the UML patch.

Ok, this makes me wonder a bit... there is something which may cause conflicts in the UML patch (i.e. renaming modify_ldt that way), but it is very *strange* that a kernel module uses the "_syscall" macro to do a syscall with "int 0x80" while already being in kernelspace...

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Marcus Better

author

2005-01-25 10:56 UTC #4 lore raw

Quoted 1 line · Blaisorblade

*strange* that a kernel module uses the "_syscall" macro to do a syscall with "int 0x80" while already being in kernelspace...

It's interesting that the module contains the following code:

------------------------
//#define FGL_USE_SCT /* for developer use only */

//...<snip>...

#ifdef FGL_USE_SCT
// get direct function pointers from sys_call_table for calling #else /* FGL_USE_SCT */
// call functions indirectly by using the syscall macros, // entrypoints get defined by below constructs

#if !defined(__ia64__)
// the macros do use errno variable
static int errno;
#endif // __ia64__

// int mlock(const void *addr, size_t len);
_syscall2(int, mlock, const void *, addr, size_t, len )
// int munlock(const void *addr, size_t len);
_syscall2(int, munlock, const void *, addr, size_t, len )
#if !defined(__ia64__)
#if !defined(__x86_64__)
// TODO: ia64
// int modify_ldt(int func, void *ptr, unsigned long bytecount); _syscall3( int, modify_ldt, int, func, void *, ptr, unsigned long, bytecount ) #endif
#endif
#endif /* FGL_USE_SCT */
--------------------------

I actually tried to #define FGL_USE_SCT, but it didn't compile either.

Blaisorblade

2005-01-25 11:03 UTC #5 lore raw

Quoted 38 lines · Marcus Better

Quoted 2 lines · Blaisorblade
*strange* that a kernel module uses the "_syscall" macro to do a syscall with "int 0x80" while already being in kernelspace...
It's interesting that the module contains the following code:
------------------------
//#define FGL_USE_SCT /* for developer use only */
//...<snip>...
#ifdef FGL_USE_SCT
// get direct function pointers from sys_call_table for calling #else /* FGL_USE_SCT */
// call functions indirectly by using the syscall macros, // entrypoints get defined by below constructs
#if !defined(__ia64__)
// the macros do use errno variable
static int errno;
#endif // __ia64__
// int mlock(const void *addr, size_t len); _syscall2(int, mlock, const void *, addr, size_t, len ) // int munlock(const void *addr, size_t len); _syscall2(int, munlock, const void *, addr, size_t, len ) #if !defined(__ia64__)
#if !defined(__x86_64__)
// TODO: ia64
// int modify_ldt(int func, void *ptr, unsigned long bytecount); _syscall3( int, modify_ldt, int, func, void *, ptr, unsigned long, bytecount )
#endif
#endif
#endif /* FGL_USE_SCT */
--------------------------

Quoted 1 line

I actually tried to #define FGL_USE_SCT, but it didn't compile either.

Ok, now I get it... they're trying to workaround the fact that modify_ldt() is not exported.

Thus they are probably violating the GPL (well, let's not start anything on this), because the syscall table has been unexported on purpose... however such tricks will break. I'll add the workaround in next revision, thanks (and if I forget, just mail me after the release).

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

A message that is not in the archive (sent off-list or not yet received): [email protected]

Blaisorblade

2005-01-28 15:52 UTC #7 lore raw

Quoted 9 lines · you

Hi,
Quoted 4 lines
Ok, now I get it... they're trying to workaround the fact that modify_ldt() is not exported.
Thus they are probably violating the GPL
I am under the impression that it is permitted to distribute a binary module, with only the "loader" under GPL.

No, I don't think so... the strict GPL interpretation is that you cannot distribute a non-GPL work which links with GPL code...

In fact, distributions cannot distribute GPL code linking with XFree 4.4.

What you probably see is that NVIDIA modules (and ATI ones too I guess, but I don't know well) have a binary-only part and a source-code part...

Linux developers mostly consider binary module to be ok until they are not "derivative works". And when a symbol is not exported, many times, it is because kernel developers feel that code using it is a derivative of the kernel.

In this case, it's maybe different - nobody thought that the sys_modify_ldt symbol should have been exported. That said, if somebody asked the question, the answer could be "No, it must not be exported, because modules using it are derivative works". On the other side, any userspace program can call the modify_ldt syscall.

Quoted 1 line

Do you mean that they are actually violating the license,

Well, just a rough feeling... I am not a lawyer.

Quoted 4 lines

or that they
are evading the kernel's tainting mechanism? I suspect that they want to
avoid users getting the message about tainted kernels.

No, users will get the message, unless the module declares it self as "GPL" with a "MODULE_LICENSE("GPL") directive in the code, which it should not.

Quoted 6 lines

If so, then I
would like to complain to them, because 1) it is disgusting behaviour and
2) it makes my life more difficult having to take care of their stupid code.

For your specific problem, I'm applying your fix to next version of the SKAS patch (which I'm publishing anyway with other work). See my other mail. Not that it is a bug of the SKAS patch or of the ATI module alone.

For the GPL issue, you should probably ask to LKML (or post a letter to LWN.net about this issue - they could maybe publish it and you'd see the comments) or on slashdot.org (which is actually a discussion forum) - I have read discussions about such issues, but I'm not myself an expert.

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Sven Köhler

2005-01-26 10:22 UTC #8 lore raw

In reply to Blaisorblade

Quoted 1 line

I.e. the compilation problem is not in the kernel module, right? In this case, the problem is that you have /usr/include/asm symlinked to your kernel's include/asm-i386 directory, which is the wrong setup. However your solution should not give any problem.

Kernel-Modules are usually compiled against the sources in either /usr/src/linux or /lib/modules/<version>/build.

Take a look at the nvidia-kernel ebuild or other ebuilds that build kernel modules. It would be fatal to compile them against the headers in /usr/include. On Gentoo, you can have linux26-headers-2.6.7 installed, but actually be running a 2.6.10 Kernel - and even worse, it is even possible to keep 2.4 headers when running a 2.6 kernel.

Blaisorblade

2005-01-26 12:25 UTC #9 lore raw

On Wednesday 26 January 2005 11:24, Sven Köhler wrote:

Please note this exact sentence:

Quoted 7 lines

Quoted 6 lines
I.e. the compilation problem is not in the kernel module, right?
In this
case, the problem is that you have /usr/include/asm symlinked to your kernel's include/asm-i386 directory, which is the wrong setup. However your solution should not give any problem.

Quoted 8 lines

Kernel-Modules are usually compiled against the sources in either /usr/src/linux or /lib/modules/<version>/build.
Take a look at the nvidia-kernel ebuild or other ebuilds that build kernel modules. It would be fatal to compile them against the headers in /usr/include. On Gentoo, you can have linux26-headers-2.6.7 installed, but actually be running a 2.6.10 Kernel - and even worse, it is even possible to keep 2.4 headers when running a 2.6 kernel.

Sven, that is clear *and* obvious... what I was *refusing* to believe was that a kernel module would be using the "syscall" macro - I explained it by thinking it was userspace code.

Maybe that use is even legitimate (1), after all, but it astonished me a bit first... and my experience with NVIDIA driver was they also had a userspace part to link with X11.

(1) legitimate means "ugly hack which can make sense with non-GPL modules, i.e. does not make sense anyway". I'm not flaming ATI / NVIDIA, let's leave this duty to people with more spare time.

Signature · 14 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Blaisorblade

2005-01-28 15:56 UTC #10 lore raw

In reply to Marcus Better

Quoted 25 lines · Marcus Better

Hi all,
I applied Blaisorblade's host-skas3-2.6.9-v7.patch, and it breaks the fglrx graphics driver from ATI[1]. This is with Debian kernel 2.6.9, but I believe it applies to vanilla kernels also.
The problem is the following declaration added to include/asm-i386/desc.h:
----------------------------------------------------------
extern int modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
        unsigned long bytecount);
----------------------------------------------------------
The fglrx driver contains the following macro in a source file:
_syscall3( int, modify_ldt, int, func, void *, ptr, unsigned long, bytecount )
Here the compiler chokes with the following error:
---------------------------------------------------------
/usr/src/modules/fglrx-4.3.0-3.14.6/firegl_public.c:181: error:
conflicting types for modify_ldt' include/asm/desc.h:144: error: previous declaration of modify_ldt'
---------------------------------------------------------

Quoted 6 lines

I don't understand the details of the kernel code, but it seems that the problem is that the SKAS patch defines the function modify_ldt which conflicts with the syscall macros.
I renamed the new function to _modify_ldt instead (and changed the two or three references to it in ldt.c), and this seems to fix the problem.

You should also have modified the reference inside arch/i386/kernel/ptrace.c.

I'm doing compilation-testing of the attached patch, which seems to be good (it is not the kind of patch easy to get wrong, however).

Signature · 3 lines

Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade

SKAS adds a function modify_ldt (closely related to sys_modify_ldt) but the declaration conflicts with some external code (ATI kernel modules). Since the name is not important, rename the function to __modify_ldt.

Signed-off-by Paolo 'Blaisorblade' Giarrusso <[email protected]>

---

 vanilla-linux-2.6.9-paolo/arch/i386/kernel/ldt.c    |    4 ++--
 vanilla-linux-2.6.9-paolo/arch/i386/kernel/ptrace.c |    2 +-
 vanilla-linux-2.6.9-paolo/include/asm-i386/desc.h   |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

include/asm-i386/desc.h renamed from include/asm-i386/desc.h~rename-modify-ldt-to-avoid-conflicts +1 -1

diff -puN include/asm-i386/desc.h~rename-modify-ldt-to-avoid-conflicts include/asm-i386/desc.h
@@ -126,7 +126,7 @@ static inline void load_LDT(mm_context_t
 	put_cpu();
 }
 
-extern int modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
+extern int __modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
 		      unsigned long bytecount);
 
 #endif /* !__ASSEMBLY__ */

arch/i386/kernel/ptrace.c renamed from arch/i386/kernel/ptrace.c~rename-modify-ldt-to-avoid-conflicts +1 -1

diff -puN arch/i386/kernel/ptrace.c~rename-modify-ldt-to-avoid-conflicts arch/i386/kernel/ptrace.c
@@ -562,7 +562,7 @@ asmlinkage int sys_ptrace(long request, 
 			ret = -EIO;
 			break;
 		}
-		ret = modify_ldt(child->mm, ldt.func, ldt.ptr, ldt.bytecount);
+		ret = __modify_ldt(child->mm, ldt.func, ldt.ptr, ldt.bytecount);
 		break;
 	}

arch/i386/kernel/ldt.c renamed from arch/i386/kernel/ldt.c~rename-modify-ldt-to-avoid-conflicts +2 -2

diff -puN arch/i386/kernel/ldt.c~rename-modify-ldt-to-avoid-conflicts arch/i386/kernel/ldt.c
@@ -238,7 +238,7 @@ out:
 	return error;
 }
 
-int modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
+int __modify_ldt(struct mm_struct * mm, int func, void __user *ptr,
 	       unsigned long bytecount)
 {
 	int ret = -ENOSYS;

@@ -262,5 +262,5 @@ int modify_ldt(struct mm_struct * mm, in
 
 asmlinkage int sys_modify_ldt(int func, void __user *ptr, unsigned long bytecount)
 {
-	return modify_ldt(current->mm, func, ptr, bytecount);
+	return __modify_ldt(current->mm, func, ptr, bytecount);
 }