Re: [PATCH 1/1] Punch a hole in /dev/mem for librtas

2011-12-04 Thread Benjamin Herrenschmidt 
On Sat, 2011-12-03 at 04:22 +0100, Segher Boessenkool wrote:
  +static inline int page_is_rtas_user_buf(unsigned long pfn)
  +{
  +   unsigned long paddr = (pfn  PAGE_SHIFT);
  +   if (paddr = rtas_rmo_buf  paddr  (rtas_rmo_buf +  
  RTAS_RMOBUF_MAX))
 
 It probably cannot overflow with actual values of rtas_rmo_buf
 and RTAS_RMOBUF_MAX, but otherwise it is an incorrect test;
 please write
 
   if (paddr = rtas_rmo_buf  paddr - rtas_rmo_buf  RTAS_RMOBUF_MAX)
 
 (and, _MAX?  Shouldn't it be the actual size here?  Or is _MAX
 just a confusing name :-) )

The original code is a lot more readable and perfectly correct for all
possible values of rtas_rmo_buf :-)

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1] Punch a hole in /dev/mem for librtas

2011-12-04 Thread Segher Boessenkool 

+static inline int page_is_rtas_user_buf(unsigned long pfn)
+{
+   unsigned long paddr = (pfn  PAGE_SHIFT);
+   if (paddr = rtas_rmo_buf  paddr  (rtas_rmo_buf +
RTAS_RMOBUF_MAX))


It probably cannot overflow with actual values of rtas_rmo_buf
and RTAS_RMOBUF_MAX, but otherwise it is an incorrect test;
please write

if (paddr = rtas_rmo_buf  paddr - rtas_rmo_buf  RTAS_RMOBUF_MAX)

(and, _MAX?  Shouldn't it be the actual size here?  Or is _MAX
just a confusing name :-) )


The original code is a lot more readable and perfectly correct for all
possible values of rtas_rmo_buf :-)


You have to consider those possible values before you see it cannot
overflow.  So no, it's a lot _less_ readable IMHO.

It's also a dangerous habit to get into.  Just say no :-)


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/1] Punch a hole in /dev/mem for librtas

2011-12-02 Thread Sukadev Bhattiprolu 

Subject: [PATCH 1/1] Punch a hole in /dev/mem for librtas

With CONFIG_STRICT_DEVMEM=y, user space cannot read any part of /dev/mem.
Since this breaks librtas, punch a hole in /dev/mem to allow access to the
rmo_buffer that librtas needs.

Anton Blanchard reported the problem and helped with the fix.

A quick test for this patch:

   # cat /proc/rtas/rmo_buffer
   0f19 1

   # python -c print 0x0f19 / 0x1
   3865

   # dd if=/dev/mem of=/tmp/foo count=1 bs=64k skip=3865
   1+0 records in
   1+0 records out
   65536 bytes (66 kB) copied, 0.000205235 s, 319 MB/s

   # dd if=/dev/mem of=/tmp/foo
   dd: reading `/dev/mem': Operation not permitted
   0+0 records in
   0+0 records out
   0 bytes (0 B) copied, 0.00022519 s, 0.0 kB/s

Changelog[v3]:  [Ben Harrenschmidt]: Incremental patch for the punched hole,
since CONFIG_STRICT_DEVMEM was merged into the -next branch.

[Ben Harrenschmidt]: Rename interface to page_is_rtas_user_buf()
move declaration to a header file and ensure it doesn't break
CONFIG_PPC_RTAS=n.

Signed-off-by: Sukadev Bhattiprolu suka...@linux.vnet.ibm.com
---
 arch/powerpc/include/asm/rtas.h |   12 
 arch/powerpc/mm/mem.c   |3 +++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 58625d1..c2d3c9f 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -305,5 +305,17 @@ static inline u32 rtas_config_addr(int busno, int devfn, 
int reg)
 extern void __cpuinit rtas_give_timebase(void);
 extern void __cpuinit rtas_take_timebase(void);
 
+#ifdef CONFIG_PPC_RTAS
+static inline int page_is_rtas_user_buf(unsigned long pfn)
+{
+   unsigned long paddr = (pfn  PAGE_SHIFT);
+   if (paddr = rtas_rmo_buf  paddr  (rtas_rmo_buf + RTAS_RMOBUF_MAX))
+   return 1;
+   return 0;
+}
+#else
+static inline int page_is_rtas_user_buf(unsigned long pfn) { return 0;}
+#endif
+
 #endif /* __KERNEL__ */
 #endif /* _POWERPC_RTAS_H */
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 553fb41..05abd49 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -50,6 +50,7 @@
 #include asm/vdso.h
 #include asm/fixmap.h
 #include asm/swiotlb.h
+#include asm/rtas.h
 
 #include mmu_decl.h
 
@@ -564,6 +565,8 @@ int devmem_is_allowed(unsigned long pfn)
return 0;
if (!page_is_ram(pfn))
return 1;
+   if (page_is_rtas_user_buf(pfn))
+   return 1;
return 0;
 }
 #endif /* CONFIG_STRICT_DEVMEM */
-- 
1.7.0.4

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 1/1] Punch a hole in /dev/mem for librtas

2011-12-02 Thread Segher Boessenkool 

+static inline int page_is_rtas_user_buf(unsigned long pfn)
+{
+   unsigned long paddr = (pfn  PAGE_SHIFT);
+	if (paddr = rtas_rmo_buf  paddr  (rtas_rmo_buf +  
RTAS_RMOBUF_MAX))


It probably cannot overflow with actual values of rtas_rmo_buf
and RTAS_RMOBUF_MAX, but otherwise it is an incorrect test;
please write

if (paddr = rtas_rmo_buf  paddr - rtas_rmo_buf  RTAS_RMOBUF_MAX)

(and, _MAX?  Shouldn't it be the actual size here?  Or is _MAX
just a confusing name :-) )


Segher

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev