On Thu, Aug 04, 2005 at 11:50:45 +0000, Vincenzo Mallozzi wrote:
>
> > As a side note: I hope you use the macros in linux/list.h, so it's not
> > a stupid thinko in the list walking code somewhere.
> >
>
> Oh! No, I'm not using the macros in linux/list.h. I've used a simple C list.
> The definition of this list is:
>
>
> 1. struct mtpmc_wrprotected_pages{
> 2. unsigned long address;
> 3. struct mtpmc_wrprotected_pages *next_page;
> 4. };
> 5.
> 6. struct mtpmc_vm_wrprotected{
> 7. unsigned long vm_start;
> 8. unsigned long vm_end;
> 9.
> 10. struct mtpmc_wrprotected_pages *pages;
> 11. struct mtpmc_vm_wrprotected *vm_next;
> 12. };
> 13.
> 14. static struct mtpmc_vm_wrprotected *mtpmc_mm_wrprotected;
>
> in which I records the vmas and the corresponding pages that I've
> write-protected.
> Now I post also the other pieces of code I'm using:
I have written something like this several times and always had a bug in
it at at least one place. Then I found linux/list.h and started using
that -- and never had a bug in it again. It's a lot more safer and
cleaner. Though it may not be the bug, I suggest you rewrite the code
using list.h -- it is quite a bit safer than by hand.
> The exception handler function hijacked:
>
> 15. static asmlinkage void mtpmc_handler(struct pt_regs * regs,
> long error_code)
> 16. {
> 17. unsigned long address;
> 18. struct mm_struct *mm;
> 19. struct mtpmc_address_fault *temp;
> 20.
> 21. unsigned long pid = current->pid;
> 22. int hijack = 0;
> 23.
> 24. /* store the old_exception handler pointer in mtpmc_old_int_handler */
> 25. void (*mtpmc_old_int_handler)(struct pt_regs *,long) =
> (void*)mtpmc_old_handler;
Why is this line actually here? Can't you call it directly from the
global variable?
> 26.
> 27. /* get the address */
> 28. __asm__("movl %%cr2,%0":"=r" (address));
> 29.
> 30. mm = current->mm;
> 31. if ((current->pid>=mtpmc_min_pid) && (current->pid<=mtpmc_max_pid))
> 32. if ((error_code & 3) == 3)
> 33. if (mtpmc_protected_by_us(address) == 1) /*ERROR IN CALLING THIS
> FUNCTION*/
> 34. {
> 35. send_sig(SIGSTOP, current, 1);
> 36. hijack = 1;*/
> 37. }
> 38.
> 39. if (hijack != 1)
> 40. (*mtpmc_old_int_handler)(regs,error_code);/*call the original handler*/
I am not actually sure this is supposed to work. asmlinkage changes the
calling convention so I would expect simply calling it to not work. But
I don't know the calling convention tricks well enough to be sure. Also
note, that in 2.6.9 it is asmlinkage, but in 2.6.11 it is fastcall.
Also are you sure %%cr2 does not get destroyed before you call this. It
shouldn't be, but anyway.
> 41.
> 42. return;
> 43. }
>
>
> The line that causes the error is the 33.th, when during the call to the
> mtpmc_protected_by_us() function. This function scan the list created by me
> in which I store the value of memory pages write-protected by me.
>
> 44. #define INSIDE(a, b, c) ( ((c) <= (b)) && ((c) >= (a)) )
> 45.
> 46. int mtpmc_protected_by_us(unsigned long addr)
> 47. {
> 48. struct mtpmc_vm_wrprotected *wr_vma;
> 49. struct mtpmc_wrprotected_pages *wr_page;
> 50.
> 51. for (wr_vma=mtpmc_mm_wrprotected; wr_vma!=NULL; wr_vma=wr_vma->vm_next)
> 52. if (INSIDE(wr_vma->vm_start, wr_vma->vm_end, addr)){
> 53. for (wr_page=wr_vma->pages; wr_page!=NULL; wr_page=wr_page->next_page)
> 54. if ((addr >= wr_page->address) && (addr <(wr_page->address +
> PAGE_SIZE)))
> 55. return 1;
> 56. return 0;
> 57. }
> 58.
> 59. return 0;
> 60. }
Now this is plain ugly. It would look quite a bit more readable if it was using
list_foreach macro.
Anyway, I suspect the problem might be the call to the original handler.
Also make sure you properly lock the list with irqsaving spinlocks (ie.
spin_lock_irqsave/spin_unlock_irqrestore outside the handler and
spin_lock/spin_unlock inside.
-------------------------------------------------------------------------------
Jan 'Bulb' Hudec <bulb@xxxxxx>
Attachment:
signature.asc
Description: Digital signature
