Re: [PATCH 2/2 resend] mm: various cleanups in get_user_pages()
Hi Nick, Thanks for the review. > On Wednesday 13 February 2008 00:10, Eugene Teo wrote: [...] > > diff --git a/mm/memory.c b/mm/memory.c > > index 54f951b..c7e0610 100644 > > --- a/mm/memory.c > > +++ b/mm/memory.c > > @@ -1003,7 +1003,9 @@ int get_user_pages(struct task_struct *tsk, struct > > mm_struct *mm, unsigned int foll_flags; > > > > vma = find_extend_vma(mm, start); > > - if (!vma && in_gate_area(tsk, start)) { > > + if (!vma) > > + goto finish_or_fault; > > + if (in_gate_area(tsk, start)) { > > unsigned long pg = start & PAGE_MASK; > > struct vm_area_struct *gate_vma = get_gate_vma(tsk); > > pgd_t *pgd; > > Doesn't this break the logic? > > If you don't have a vma, but you are in the gate area, then you > should use the gate vma. With your patch, gate area will fault. Yes, you are right. I also relooked at the patch, and actually vma is validated after if (... in_gate_area(tsk, start)) { ... }, so my patch is not correct. > > @@ -1011,7 +1013,7 @@ int get_user_pages(struct task_struct *tsk, struct > > mm_struct *mm, pmd_t *pmd; > > pte_t *pte; > > if (write) /* user gate pages are read-only */ > > - return i ? : -EFAULT; > > + goto finish_or_fault; > > I don't know if this is exactly a cleanup or not... I guess gcc > probably isn't smart enough to fold them all together, so it should > use a little less code in the unlikely branches. Does it? Agree. Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2 resend] mm: various cleanups in get_user_pages()
Sorry for the repeated emails. Kindly ignore the previous resend. Please review this instead. Thanks. I have tested this. [PATCH 2/2] mm: various cleanups in get_user_pages() This patch contains various cleanups, including making sure vma is valid, and the return value of follow_hugetlb_page() is validated. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- mm/memory.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 54f951b..c7e0610 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1003,7 +1003,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, unsigned int foll_flags; vma = find_extend_vma(mm, start); - if (!vma && in_gate_area(tsk, start)) { + if (!vma) + goto finish_or_fault; + if (in_gate_area(tsk, start)) { unsigned long pg = start & PAGE_MASK; struct vm_area_struct *gate_vma = get_gate_vma(tsk); pgd_t *pgd; @@ -1011,7 +1013,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, pmd_t *pmd; pte_t *pte; if (write) /* user gate pages are read-only */ - return i ? : -EFAULT; + goto finish_or_fault; if (pg > TASK_SIZE) pgd = pgd_offset_k(pg); else @@ -1021,11 +1023,11 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, BUG_ON(pud_none(*pud)); pmd = pmd_offset(pud, pg); if (pmd_none(*pmd)) - return i ? : -EFAULT; + goto finish_or_fault; pte = pte_offset_map(pmd, pg); if (pte_none(*pte)) { pte_unmap(pte); - return i ? : -EFAULT; + goto finish_or_fault; } if (pages) { struct page *page = vm_normal_page(gate_vma, start, *pte); @@ -1041,9 +1043,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, continue; } - if (!vma || (vma->vm_flags & (VM_IO | VM_PFNMAP)) + if ((vma->vm_flags & (VM_IO | VM_PFNMAP)) || !(vm_flags & vma->vm_flags)) - return i ? : -EFAULT; + goto finish_or_fault; if (is_vm_hugetlb_page(vma)) { i = follow_hugetlb_page(mm, vma, pages, vmas, @@ -1080,9 +1082,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, foll_flags & FOLL_WRITE); if (ret & VM_FAULT_ERROR) { if (ret & VM_FAULT_OOM) - return i ? i : -ENOMEM; + goto finish_or_oom; else if (ret & VM_FAULT_SIGBUS) - return i ? i : -EFAULT; + goto finish_or_fault; BUG(); } if (ret & VM_FAULT_MAJOR) @@ -1115,6 +1117,12 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, } } return i; + +finish_or_oom: + return i ? : -ENOMEM; + +finish_or_fault: + return i ? : -EFAULT; } EXPORT_SYMBOL(get_user_pages); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2 resend] mm: various cleanups in get_user_pages()
Argh. Sorry, I spotted a mistake. Here's a resend: [PATCH 2/2] mm: various cleanups in get_user_pages() This patch contains various cleanups, including making sure vma is valid, and the return value of follow_hugetlb_page() is validated. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- mm/memory.c | 26 ++ 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 54f951b..77105c4 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1003,7 +1003,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, unsigned int foll_flags; vma = find_extend_vma(mm, start); - if (!vma && in_gate_area(tsk, start)) { + if (!vma) + goto finish_or_fault; + if (in_gate_area(tsk, start)) { unsigned long pg = start & PAGE_MASK; struct vm_area_struct *gate_vma = get_gate_vma(tsk); pgd_t *pgd; @@ -1011,7 +1013,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, pmd_t *pmd; pte_t *pte; if (write) /* user gate pages are read-only */ - return i ? : -EFAULT; + goto finish_or_fault; if (pg > TASK_SIZE) pgd = pgd_offset_k(pg); else @@ -1021,11 +1023,11 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, BUG_ON(pud_none(*pud)); pmd = pmd_offset(pud, pg); if (pmd_none(*pmd)) - return i ? : -EFAULT; + goto finish_or_fault; pte = pte_offset_map(pmd, pg); if (pte_none(*pte)) { pte_unmap(pte); - return i ? : -EFAULT; + goto finish_or_fault; } if (pages) { struct page *page = vm_normal_page(gate_vma, start, *pte); @@ -1041,13 +1043,15 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, continue; } - if (!vma || (vma->vm_flags & (VM_IO | VM_PFNMAP)) + if ((vma->vm_flags & (VM_IO | VM_PFNMAP)) || !(vm_flags & vma->vm_flags)) - return i ? : -EFAULT; + goto finish_or_fault; if (is_vm_hugetlb_page(vma)) { i = follow_hugetlb_page(mm, vma, pages, vmas, &start, len, i, write); + if (i == -EFAULT) + return -EFAULT; continue; } @@ -1080,9 +1084,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, foll_flags & FOLL_WRITE); if (ret & VM_FAULT_ERROR) { if (ret & VM_FAULT_OOM) - return i ? i : -ENOMEM; + goto finish_or_oom; else if (ret & VM_FAULT_SIGBUS) - return i ? i : -EFAULT; + goto finish_or_fault; BUG(); } if (ret & VM_FAULT_MAJOR) @@ -1115,6 +1119,12 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, } } return i; + +finish_or_oom: + return i ? : -ENOMEM; + +finish_or_fault: + return i ? : -EFAULT; } EXPORT_SYMBOL(get_user_pages); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] mm: various cleanups in get_user_pages()
This patch contains various cleanups, including making sure vma is valid, and the return value of follow_hugetlb_page() is validated. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- mm/memory.c | 26 ++ 1 files changed, 18 insertions(+), 8 deletions(-) diff --git a/mm/memory.c b/mm/memory.c index 54f951b..49403a8 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1003,7 +1003,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, unsigned int foll_flags; vma = find_extend_vma(mm, start); - if (!vma && in_gate_area(tsk, start)) { + if (!vma) + goto finish_or_fault; + if (in_gate_area(tsk, start)) { unsigned long pg = start & PAGE_MASK; struct vm_area_struct *gate_vma = get_gate_vma(tsk); pgd_t *pgd; @@ -1011,7 +1013,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, pmd_t *pmd; pte_t *pte; if (write) /* user gate pages are read-only */ - return i ? : -EFAULT; + goto finish_or_fault; if (pg > TASK_SIZE) pgd = pgd_offset_k(pg); else @@ -1021,11 +1023,11 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, BUG_ON(pud_none(*pud)); pmd = pmd_offset(pud, pg); if (pmd_none(*pmd)) - return i ? : -EFAULT; + goto finish_or_fault; pte = pte_offset_map(pmd, pg); if (pte_none(*pte)) { pte_unmap(pte); - return i ? : -EFAULT; + goto finish_or_fault; } if (pages) { struct page *page = vm_normal_page(gate_vma, start, *pte); @@ -1041,13 +1043,15 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, continue; } - if (!vma || (vma->vm_flags & (VM_IO | VM_PFNMAP)) + if ((vma->vm_flags & (VM_IO | VM_PFNMAP)) || !(vm_flags & vma->vm_flags)) - return i ? : -EFAULT; + goto finish_or_fault; if (is_vm_hugetlb_page(vma)) { i = follow_hugetlb_page(mm, vma, pages, vmas, &start, len, i, write); + if (i == -EFAULT) + goto finish_or_fault; continue; } @@ -1080,9 +1084,9 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, foll_flags & FOLL_WRITE); if (ret & VM_FAULT_ERROR) { if (ret & VM_FAULT_OOM) - return i ? i : -ENOMEM; + goto finish_or_oom; else if (ret & VM_FAULT_SIGBUS) - return i ? i : -EFAULT; + goto finish_or_fault; BUG(); } if (ret & VM_FAULT_MAJOR) @@ -1115,6 +1119,12 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, } } return i; + +finish_or_oom: + return i ? : -ENOMEM; + +finish_or_fault: + return i ? : -EFAULT; } EXPORT_SYMBOL(get_user_pages); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] mm: make get_user_pages() more robust in handling arguments
Ensure that get_user_pages() evaluates len upon entry into the while loops. A BUG_ON check is added so that it will catch potential bugs when it is asked to grab zero pages. follow_hugetlb_page() is modified to adapt the changes made in get_user_pages(). Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- include/linux/hugetlb.h |2 +- mm/hugetlb.c| 10 +++--- mm/memory.c | 15 ++- 3 files changed, 10 insertions(+), 17 deletions(-) diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 7ca198b..2e8a01d 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -20,7 +20,7 @@ int hugetlb_sysctl_handler(struct ctl_table *, int, struct file *, void __user * int hugetlb_overcommit_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int hugetlb_treat_movable_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *); -int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int *, int, int); +int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int, int, int); void unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); void __unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); int hugetlb_prefault(struct address_space *, struct vm_area_struct *); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index d9a3803..5bb8130 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -960,15 +960,14 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, struct page **pages, struct vm_area_struct **vmas, - unsigned long *position, int *length, int i, + unsigned long *position, int length, int i, int write) { unsigned long pfn_offset; unsigned long vaddr = *position; - int remainder = *length; spin_lock(&mm->page_table_lock); - while (vaddr < vma->vm_end && remainder) { + while (i < length && vaddr < vma->vm_end) { pte_t *pte; struct page *page; @@ -988,7 +987,6 @@ int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, if (!(ret & VM_FAULT_ERROR)) continue; - remainder = 0; if (!i) i = -EFAULT; break; @@ -1007,9 +1005,8 @@ same_page: vaddr += PAGE_SIZE; ++pfn_offset; - --remainder; ++i; - if (vaddr < vma->vm_end && remainder && + if (i < length && vaddr < vma->vm_end && pfn_offset < HPAGE_SIZE/PAGE_SIZE) { /* * We use pfn_offset to avoid touching the pageframes @@ -1019,7 +1016,6 @@ same_page: } } spin_unlock(&mm->page_table_lock); - *length = remainder; *position = vaddr; return i; diff --git a/mm/memory.c b/mm/memory.c index 717aa0e..54f951b 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -989,8 +989,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, int i; unsigned int vm_flags; - if (len <= 0) - return 0; + BUG_ON(len <= 0); /* * Require read or write permissions. * If 'force' is set, we only require the "MAY" flags. @@ -999,7 +998,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, vm_flags &= force ? (VM_MAYREAD | VM_MAYWRITE) : (VM_READ | VM_WRITE); i = 0; - do { + while (i < len) { struct vm_area_struct *vma; unsigned int foll_flags; @@ -1039,7 +1038,6 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, vmas[i] = gate_vma; i++; start += PAGE_SIZE; - len--; continue; } @@ -1049,7 +1047,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, if (is_vm_hugetlb_page(vma)) { i = follow_hugetlb_page(mm, vma, pages, vmas, - &start, &len, i, write); + &start, len, i, write); continue; }
Re: [PATCH] mm: tidy up follow_hugetlb_page() and get_user_pages()
> On Tue, 12 Feb 2008 13:28:40 +0800 Eugene Teo <[EMAIL PROTECTED]> wrote: > > > This patch extends Jonathan Corbet's patch to avoid buffer overflows in > > get_user_pages(). It cleans up follow_hugetlb_page(), and get_user_pages() > > so > > that it is easier to read. It also makes sure that len and vma are > > validated. > > We'd prefer that the functional change not be bundled with a (large) cleanup, > please. Ok. I will split them up, and resend. Thanks, Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mm: tidy up follow_hugetlb_page() and get_user_pages()
Hi, I noticed that Linus has committed your patch. Here's a resend of my patch. Kindly review please. [PATCH] mm: tidy up follow_hugetlb_page() and get_user_pages() This patch extends Jonathan Corbet's patch to avoid buffer overflows in get_user_pages(). It tidies up follow_hugetlb_page(), and get_user_pages() to make sure that vma is also validated, and the code is more readable. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- include/linux/hugetlb.h |2 +- mm/hugetlb.c| 13 +++-- mm/memory.c | 39 ++- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 7ca198b..2e8a01d 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -20,7 +20,7 @@ int hugetlb_sysctl_handler(struct ctl_table *, int, struct file *, void __user * int hugetlb_overcommit_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int hugetlb_treat_movable_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *); -int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int *, int, int); +int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int, int, int); void unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); void __unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); int hugetlb_prefault(struct address_space *, struct vm_area_struct *); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index d9a3803..ac5cf8a 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -960,15 +960,14 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, struct page **pages, struct vm_area_struct **vmas, - unsigned long *position, int *length, int i, + unsigned long *position, int length, int i, int write) { unsigned long pfn_offset; unsigned long vaddr = *position; - int remainder = *length; spin_lock(&mm->page_table_lock); - while (vaddr < vma->vm_end && remainder) { + while (i < length && vaddr < vma->vm_end) { pte_t *pte; struct page *page; @@ -987,10 +986,6 @@ int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, spin_lock(&mm->page_table_lock); if (!(ret & VM_FAULT_ERROR)) continue; - - remainder = 0; - if (!i) - i = -EFAULT; break; } @@ -1007,9 +1002,8 @@ same_page: vaddr += PAGE_SIZE; ++pfn_offset; - --remainder; ++i; - if (vaddr < vma->vm_end && remainder && + if (i < length && vaddr < vma->vm_end && pfn_offset < HPAGE_SIZE/PAGE_SIZE) { /* * We use pfn_offset to avoid touching the pageframes @@ -1019,7 +1013,6 @@ same_page: } } spin_unlock(&mm->page_table_lock); - *length = remainder; *position = vaddr; return i; diff --git a/mm/memory.c b/mm/memory.c index 717aa0e..ac7d104 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -989,8 +989,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, int i; unsigned int vm_flags; - if (len <= 0) - return 0; + BUG_ON(len <= 0); /* * Require read or write permissions. * If 'force' is set, we only require the "MAY" flags. @@ -999,12 +998,14 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, vm_flags &= force ? (VM_MAYREAD | VM_MAYWRITE) : (VM_READ | VM_WRITE); i = 0; - do { + while (i < len) { struct vm_area_struct *vma; unsigned int foll_flags; vma = find_extend_vma(mm, start); - if (!vma && in_gate_area(tsk, start)) { + if (!vma) + goto finish_or_fault; + if (in_gate_area(tsk, start)) { unsigned long pg = start & PAGE_MASK; struct vm_area_struct *gate_vma = get_gate_vma(tsk); pgd_t *pgd; @@ -1012,7 +1013,7 @@ int get_user_pages(struct tas
[PATCH] mm: tidy up follow_hugetlb_page() and get_user_pages()
This patch extends Jonathan Corbet's patch to avoid buffer overflows in get_user_pages(). It cleans up follow_hugetlb_page(), and get_user_pages() so that it is easier to read. It also makes sure that len and vma are validated. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- include/linux/hugetlb.h |2 +- mm/hugetlb.c| 13 +++-- mm/memory.c | 39 +++ 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h index 30d606a..cf6c33e 100644 --- a/include/linux/hugetlb.h +++ b/include/linux/hugetlb.h @@ -19,7 +19,7 @@ static inline int is_vm_hugetlb_page(struct vm_area_struct *vma) int hugetlb_sysctl_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int hugetlb_treat_movable_handler(struct ctl_table *, int, struct file *, void __user *, size_t *, loff_t *); int copy_hugetlb_page_range(struct mm_struct *, struct mm_struct *, struct vm_area_struct *); -int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int *, int, int); +int follow_hugetlb_page(struct mm_struct *, struct vm_area_struct *, struct page **, struct vm_area_struct **, unsigned long *, int, int, int); void unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); void __unmap_hugepage_range(struct vm_area_struct *, unsigned long, unsigned long); int hugetlb_prefault(struct address_space *, struct vm_area_struct *); diff --git a/mm/hugetlb.c b/mm/hugetlb.c index 1a56420..f34076f 100644 --- a/mm/hugetlb.c +++ b/mm/hugetlb.c @@ -950,15 +950,14 @@ int hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma, int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, struct page **pages, struct vm_area_struct **vmas, - unsigned long *position, int *length, int i, + unsigned long *position, int length, int i, int write) { unsigned long pfn_offset; unsigned long vaddr = *position; - int remainder = *length; spin_lock(&mm->page_table_lock); - while (vaddr < vma->vm_end && remainder) { + for (; i < length && vaddr < vma->vm_end; ) { pte_t *pte; struct page *page; @@ -977,10 +976,6 @@ int follow_hugetlb_page(struct mm_struct *mm, struct vm_area_struct *vma, spin_lock(&mm->page_table_lock); if (!(ret & VM_FAULT_ERROR)) continue; - - remainder = 0; - if (!i) - i = -EFAULT; break; } @@ -997,9 +992,8 @@ same_page: vaddr += PAGE_SIZE; ++pfn_offset; - --remainder; ++i; - if (vaddr < vma->vm_end && remainder && + if (i < length && vaddr < vma->vm_end && pfn_offset < HPAGE_SIZE/PAGE_SIZE) { /* * We use pfn_offset to avoid touching the pageframes @@ -1009,7 +1003,6 @@ same_page: } } spin_unlock(&mm->page_table_lock); - *length = remainder; *position = vaddr; return i; diff --git a/mm/memory.c b/mm/memory.c index 153a54b..6221b03 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -991,20 +991,23 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, int i; unsigned int vm_flags; + BUG_ON(len <= 0); + /* * Require read or write permissions. * If 'force' is set, we only require the "MAY" flags. */ vm_flags = write ? (VM_WRITE | VM_MAYWRITE) : (VM_READ | VM_MAYREAD); vm_flags &= force ? (VM_MAYREAD | VM_MAYWRITE) : (VM_READ | VM_WRITE); - i = 0; - do { + for (i = 0; i < len; ) { struct vm_area_struct *vma; unsigned int foll_flags; vma = find_extend_vma(mm, start); - if (!vma && in_gate_area(tsk, start)) { + if (!vma) + goto finish_or_fault; + if (in_gate_area(tsk, start)) { unsigned long pg = start & PAGE_MASK; struct vm_area_struct *gate_vma = get_gate_vma(tsk); pgd_t *pgd; @@ -1012,7 +1015,7 @@ int get_user_pages(struct task_struct *tsk, struct mm_struct *mm, pmd_t *pmd; pte_t *pte; if (write) /* user gate pages are read-only */ - return i ?
Re: [PATCH] proc: extend /proc//fdinfo/
Hi Motohiro-san, > In general, I think this patch isn't wrong idea. > but it shuld be brush up more, may be. Thanks. > > kerndev: ~/code/kernel# cat /proc/`pgrep pickup`/fdinfo/6 > > mode: 0622 > > I think this is inode attribute, but not fd attribute. Yes, it is an inode attribute. > > dev:253,0 > > ino:21463057 > > may be useful, agreed with you :) Thanks. > > uid:89 > > gid:89 > > rdev: 0,0 > > pos:0 > > flags: 04002 FD_CLOEXEC # if close-on-exec flag is set > > agreed with Miklos's opinion. > > > path: /var/spool/postfix/public/pickup > > may be, we shold think namespace.. Besides readlink, this info can be gathered by running: ls -laF /proc//fd/ So, I agree with Miklo's opinion that we can omit this in the output. > > You can also use this to find out more information about locked open files: > > is your requirement only fd -> ino pick up? Not only this, but it is useful to know quickly if the open file descriptor has a close-on-exec flag set. Thanks, Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] proc: extend /proc//fdinfo/
Hi Miklos, > On Sat, 2008-02-09 at 20:01 +0800, Eugene Teo wrote: > > This patch extends /proc//fdinfo/ to report information about open > > files, and pathname. This information can be useful to know when debugging > > an > > application. > > > > For each file descriptor, the information is provided in the following > > format: > > > > kerndev: ~/code/kernel# cat /proc/`pgrep pickup`/fdinfo/6 > > mode: 0622 > > dev:253,0 > > ino:21463057 > > These are already available via 'stat /proc//fd/' > > > uid:89 > > gid:89 > > AFAICS file->f_[ug]id are only used by the netfilter code, but I guess > it wouldn't hurt to show them here. But please add fields at the end of > the file, instead of the beggining. Ok, will do. > > rdev: 0,0 > > This is in struct stat too. > > > pos:0 > > flags: 04002 FD_CLOEXEC # if close-on-exec flag is set > > FD_CLOEXEC should be on a separate line, because it's not an O_FOO flag. > > path: /var/spool/postfix/public/pickup > > readlink /proc//fd/ Thanks for the review. I agree that some of the information can be omitted since it can be accessed using stat and readlink. I still think it is useful to have ino in the output though. Here's the updated patch: [PATCH] proc: extend /proc//fdinfo/ This patch extends /proc//fdinfo/ to report information about open files, and pathname. This information can be useful to know when debugging an application. For each file descriptor, the information is provided in the following format: kerndev: ~/code/kernel$ cat /proc/`pgrep firefox-bin`/fdinfo/29 pos:49152 flags: 02 uid:500 gid:500 ino:13205793 fd_flags: FD_CLOEXEC # if close-on-exec flag is set Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/base.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index c59852b..5ea5dee 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1402,7 +1402,7 @@ out: return ~0U; } -#define PROC_FDINFO_MAX 64 +#define PROC_FDINFO_MAX 128 static int proc_fd_info(struct inode *inode, struct dentry **dentry, struct vfsmount **mnt, char *info) @@ -1428,12 +1428,26 @@ static int proc_fd_info(struct inode *inode, struct dentry **dentry, *mnt = mntget(file->f_path.mnt); if (dentry) *dentry = dget(file->f_path.dentry); - if (info) + /* called by proc_fdinfo_read() */ + if (info) { + struct fdtable *fdt = files_fdtable(files); + struct dentry *d = dget(file->f_path.dentry); + int gcoe = FD_ISSET(fd, fdt->close_on_exec); + snprintf(info, PROC_FDINFO_MAX, "pos:\t%lli\n" -"flags:\t0%o\n", +"flags:\t0%o\n" +"uid:\t%d\n" +"gid:\t%d\n" +"ino:\t%ld\n" +"fd_flags:\t%s\n", (long long) file->f_pos, -file->f_flags); +file->f_flags, +file->f_uid, file->f_gid, +d->d_inode->i_ino, +gcoe ? "FD_CLOEXEC" : "0"); + dput(d); + } spin_unlock(&files->file_lock); put_files_struct(files); return 0; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ftrace: remove unused tracing_sched_switch_enabled
> tracing_sched_switch_enabled isn't used anywhere. > > Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> Sorry, I forgot to remove the variable declaration. Here's a resend: tracing_sched_switch_enabled isn't used anywhere. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- kernel/trace/trace.h |1 - kernel/trace/trace_sched_switch.c | 11 +-- 2 files changed, 1 insertions(+), 11 deletions(-) diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index ac851b2..3173a93 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -138,7 +138,6 @@ void unregister_tracer(struct tracer *type); extern unsigned long nsecs_to_usecs(unsigned long nsecs); -extern int tracing_sched_switch_enabled; extern unsigned long tracing_max_latency; extern unsigned long tracing_thresh; diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c index 412c226..3e4771d 100644 --- a/kernel/trace/trace_sched_switch.c +++ b/kernel/trace/trace_sched_switch.c @@ -16,7 +16,6 @@ static struct trace_array *ctx_trace; static int __read_mostly tracer_enabled; -int __read_mostly tracing_sched_switch_enabled; static void notrace ctx_switch_func(struct task_struct *prev, struct task_struct *next) @@ -112,14 +111,6 @@ static struct tracer sched_switch_trace __read_mostly = __init static int init_sched_switch_trace(void) { - int ret; - - ret = register_tracer(&sched_switch_trace); - if (ret) - return ret; - - tracing_sched_switch_enabled = 1; - - return ret; + return register_tracer(&sched_switch_trace); } device_initcall(init_sched_switch_trace); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] ftrace: remove unused tracing_sched_switch_enabled
tracing_sched_switch_enabled isn't used anywhere. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- kernel/trace/trace.h |1 - kernel/trace/trace_sched_switch.c |9 + 2 files changed, 1 insertions(+), 9 deletions(-) diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h index ac851b2..3173a93 100644 --- a/kernel/trace/trace.h +++ b/kernel/trace/trace.h @@ -138,7 +138,6 @@ void unregister_tracer(struct tracer *type); extern unsigned long nsecs_to_usecs(unsigned long nsecs); -extern int tracing_sched_switch_enabled; extern unsigned long tracing_max_latency; extern unsigned long tracing_thresh; diff --git a/kernel/trace/trace_sched_switch.c b/kernel/trace/trace_sched_switch.c index 412c226..c4e0134 100644 --- a/kernel/trace/trace_sched_switch.c +++ b/kernel/trace/trace_sched_switch.c @@ -16,7 +16,6 @@ static struct trace_array *ctx_trace; static int __read_mostly tracer_enabled; -int __read_mostly tracing_sched_switch_enabled; static void notrace ctx_switch_func(struct task_struct *prev, struct task_struct *next) @@ -114,12 +113,6 @@ __init static int init_sched_switch_trace(void) { int ret; - ret = register_tracer(&sched_switch_trace); - if (ret) - return ret; - - tracing_sched_switch_enabled = 1; - - return ret; + return register_tracer(&sched_switch_trace); } device_initcall(init_sched_switch_trace); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [12/19] ftrace: function tracer
> From: Steven Rostedt <[EMAIL PROTECTED]> > > This is a simple trace that uses the ftrace infrastructure. It is > designed to be fast and small, and easy to use. It is useful to > record things that happen over a very short period of time, and > not to analyze the system in general. > > Updates: > > available_tracers > "function" is added to this file. $ cat available_tracers wakeup irqsoff ftrace sched_switch none ^^ I believe "function" refers to "ftrace"? [...] > echo "symonly" > /debugfs/tracing/iter_ctrl > > tracer: > [ 81.479913] CPU 0: bash:3154 register_ftrace_function+0x5f/0x66 <-- > _spin_unlock_irqrestore+0xe/0x5a > [ 81.479913] CPU 0: bash:3154 _spin_unlock_irqrestore+0x3e/0x5a <-- > sub_preempt_count+0xc/0x7a > [ 81.479913] CPU 0: bash:3154 sub_preempt_count+0x30/0x7a <-- > in_lock_functions+0x9/0x24 > [ 81.479914] CPU 0: bash:3154 vfs_write+0x11d/0x155 <-- > dnotify_parent+0x12/0x78 > [ 81.479914] CPU 0: bash:3154 dnotify_parent+0x2d/0x78 <-- > _spin_lock+0xe/0x70 > [ 81.479914] CPU 0: bash:3154 _spin_lock+0x1b/0x70 <-- > add_preempt_count+0xe/0x77 > [ 81.479914] CPU 0: bash:3154 add_preempt_count+0x3e/0x77 <-- > in_lock_functions+0x9/0x24 I can't seem to echo "symonly" to iter_ctrl. kerndev: /sys/kernel/debug/tracing$ cat current_tracer ftrace kerndev: /sys/kernel/debug/tracing$ cat tracing_enabled 1 kerndev: /sys/kernel/debug/tracing$ cat iter_ctrl noprint-parent nosym-offset nosym-addr noverbose kerndev: /sys/kernel/debug/tracing$ echo symonly > iter_ctrl kerndev: /sys/kernel/debug/tracing$ cat iter_ctrl noprint-parent nosym-offset nosym-addr noverbose How did you get the above output? Thanks, Eugene -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] lguest: make sure cpu is initialized before accessing it
If req is LHREQ_INITIALIZE, and the guest has been initialized before (unlikely), it will attempt to access cpu->tsk even though cpu is not yet initialized. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/lguest/lguest_user.c | 14 +++--- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/lguest/lguest_user.c b/drivers/lguest/lguest_user.c index 85d42d3..9cbb285 100644 --- a/drivers/lguest/lguest_user.c +++ b/drivers/lguest/lguest_user.c @@ -241,15 +241,15 @@ static ssize_t write(struct file *file, const char __user *in, cpu = &lg->cpus[cpu_id]; if (!cpu) return -EINVAL; - } - /* Once the Guest is dead, all you can do is read() why it died. */ - if (lg && lg->dead) - return -ENOENT; + /* Once the Guest is dead, all you can do is read() why it died. */ + if (lg && lg->dead) + return -ENOENT; - /* If you're not the task which owns the Guest, you can only break */ - if (lg && current != cpu->tsk && req != LHREQ_BREAK) - return -EPERM; + /* If you're not the task which owns the Guest, you can only break */ + if (lg && current != cpu->tsk && req != LHREQ_BREAK) + return -EPERM; + } switch (req) { case LHREQ_INITIALIZE: -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] proc: extend /proc//fdinfo/
This patch extends /proc//fdinfo/ to report information about open files, and pathname. This information can be useful to know when debugging an application. For each file descriptor, the information is provided in the following format: kerndev: ~/code/kernel# cat /proc/`pgrep pickup`/fdinfo/6 mode: 0622 dev:253,0 ino:21463057 uid:89 gid:89 rdev: 0,0 pos:0 flags: 04002 FD_CLOEXEC # if close-on-exec flag is set path: /var/spool/postfix/public/pickup You can also use this to find out more information about locked open files: kerndev: /proc# cat /proc/locks 1: POSIX ADVISORY WRITE 2663 fd:00:21398205 0 EOF 2: FLOCK ADVISORY WRITE 2654 fd:00:21398203 0 EOF 3: FLOCK ADVISORY WRITE 2564 fd:00:21463056 0 EOF 4: FLOCK ADVISORY WRITE 2266 fd:00:21398151 0 EOF kerndev: /proc# cd 2663/fdinfo/ kerndev: /proc/2663/fdinfo# grep 21398205 * -B2 -A6 3-mode: 0644 3-dev: 253,0 3:ino: 21398205 3-uid: 0 3-gid: 0 3-rdev: 0,0 3-pos: 5 3-flags:02 FD_CLOEXEC 3-path: /var/run/atd.pid Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/base.c | 47 +-- 1 files changed, 41 insertions(+), 6 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index c59852b..dde617f 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1402,7 +1402,7 @@ out: return ~0U; } -#define PROC_FDINFO_MAX 64 +#define PROC_FDINFO_MAX 256 static int proc_fd_info(struct inode *inode, struct dentry **dentry, struct vfsmount **mnt, char *info) @@ -1411,6 +1411,7 @@ static int proc_fd_info(struct inode *inode, struct dentry **dentry, struct files_struct *files = NULL; struct file *file; int fd = proc_fd(inode); + int ret = -ENOENT; if (task) { files = get_files_struct(task); @@ -1424,24 +1425,58 @@ static int proc_fd_info(struct inode *inode, struct dentry **dentry, spin_lock(&files->file_lock); file = fcheck_files(files, fd); if (file) { + ret = 0; + if (mnt) *mnt = mntget(file->f_path.mnt); if (dentry) *dentry = dget(file->f_path.dentry); - if (info) + /* called by proc_fdinfo_read() */ + if (info) { + struct fdtable *fdt = files_fdtable(files); + struct dentry *d = dget(file->f_path.dentry); + struct vfsmount *v = mntget(file->f_path.mnt); + char *page = (char *)__get_free_page(GFP_TEMPORARY); + int gcoe = FD_ISSET(fd, fdt->close_on_exec); + int dev_nr = d->d_inode->i_sb->s_dev; + int rdev_nr = d->d_inode->i_rdev; + + if (!page) { + ret = -ENOMEM; + goto out; + } + snprintf(info, PROC_FDINFO_MAX, +"mode:\t0%o\n" +"dev:\t%d,%d\n" +"ino:\t%ld\n" +"uid:\t%d\n" +"gid:\t%d\n" +"rdev:\t%d,%d\n" "pos:\t%lli\n" -"flags:\t0%o\n", +"flags:\t0%o %s\n" +"path:\t%s\n", +d->d_inode->i_mode & 0777, +MAJOR(dev_nr), MINOR(dev_nr), +d->d_inode->i_ino, +file->f_uid, file->f_gid, +MAJOR(rdev_nr), MINOR(rdev_nr), (long long) file->f_pos, -file->f_flags); +file->f_flags, gcoe ? "FD_CLOEXEC" : "", +d_path(d, v, page, PAGE_SIZE)); + free_page((unsigned long)page); +out: + mntput(v); + dput(d); + } spin_unlock(&files->file_lock); put_files_struct(files); - retu
[PATCH] proc: Add RLIMIT_RTTIME to /proc//limits
RLIMIT_RTTIME was introduced to allow the user to set a runtime timeout on real-time tasks: http://lkml.org/lkml/2007/12/18/218. This patch updates /proc//limits with the new rlimit. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/base.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index c59852b..dcf7be8 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -412,6 +412,7 @@ static const struct limit_names lnames[RLIM_NLIMITS] = { [RLIMIT_MSGQUEUE] = {"Max msgqueue size", "bytes"}, [RLIMIT_NICE] = {"Max nice priority", NULL}, [RLIMIT_RTPRIO] = {"Max realtime priority", NULL}, + [RLIMIT_RTTIME] = {"Max realtime timeout", "us"}, }; /* Display limits for a process */ -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clean up exports in fs/{open,read_write}.c
Takashi-san fixed sound/isa/wavefront/wavefront_synth.c to use request_firmware instead of sys_*. Since that is the last driver in the kernel that uses sys_{read,close}, this patch kills these exports. sys_open is left exported for sparc64 only. Cc: Takashi Iwai <[EMAIL PROTECTED]> Cc: Christoph Hellwig <[EMAIL PROTECTED]> Cc: Arjan van de Ven <[EMAIL PROTECTED]> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/open.c |4 ++-- fs/read_write.c |1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/fs/open.c b/fs/open.c index 1d9e5e9..dc121ce 100644 --- a/fs/open.c +++ b/fs/open.c @@ -1056,7 +1056,9 @@ asmlinkage long sys_open(const char __user *filename, int flags, int mode) prevent_tail_call(ret); return ret; } +#ifdef CONFIG_SPARC64 EXPORT_SYMBOL_GPL(sys_open); +#endif asmlinkage long sys_openat(int dfd, const char __user *filename, int flags, int mode) @@ -1148,8 +1150,6 @@ out_unlock: return -EBADF; } -EXPORT_SYMBOL(sys_close); - /* * This routine simulates a hangup on the tty, to arrange that users * are given clean terminals at login time. diff --git a/fs/read_write.c b/fs/read_write.c index 507ddff..d913d1e 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -370,7 +370,6 @@ asmlinkage ssize_t sys_read(unsigned int fd, char __user * buf, size_t count) return ret; } -EXPORT_SYMBOL_GPL(sys_read); asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count) { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] alsa: replace calls to sys_* with filp_open and vfs_read
This patch replaces calls to sys_* with filp_open and vfs_read. And since this is the last driver in the kernel that uses sys_{read,close}, it kills the exports as well. sys_open is left exported for sparc64 only. Cc: Takashi Iwai <[EMAIL PROTECTED]> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> Signed-off-by: Arjan van de Ven <[EMAIL PROTECTED]> --- fs/open.c |4 +- fs/read_write.c |1 - sound/isa/wavefront/wavefront_synth.c | 47 +++- 3 files changed, 24 insertions(+), 28 deletions(-) diff --git a/fs/open.c b/fs/open.c index 1d9e5e9..dc121ce 100644 --- a/fs/open.c +++ b/fs/open.c @@ -1056,7 +1056,9 @@ asmlinkage long sys_open(const char __user *filename, int flags, int mode) prevent_tail_call(ret); return ret; } +#ifdef CONFIG_SPARC64 EXPORT_SYMBOL_GPL(sys_open); +#endif asmlinkage long sys_openat(int dfd, const char __user *filename, int flags, int mode) @@ -1148,8 +1150,6 @@ out_unlock: return -EBADF; } -EXPORT_SYMBOL(sys_close); - /* * This routine simulates a hangup on the tty, to arrange that users * are given clean terminals at login time. diff --git a/fs/read_write.c b/fs/read_write.c index 507ddff..d913d1e 100644 --- a/fs/read_write.c +++ b/fs/read_write.c @@ -370,7 +370,6 @@ asmlinkage ssize_t sys_read(unsigned int fd, char __user * buf, size_t count) return ret; } -EXPORT_SYMBOL_GPL(sys_read); asmlinkage ssize_t sys_write(unsigned int fd, const char __user * buf, size_t count) { diff --git a/sound/isa/wavefront/wavefront_synth.c b/sound/isa/wavefront/wavefront_synth.c index bacc51c..40eefa2 100644 --- a/sound/isa/wavefront/wavefront_synth.c +++ b/sound/isa/wavefront/wavefront_synth.c @@ -1941,8 +1941,6 @@ wavefront_reset_to_cleanliness (snd_wavefront_t *dev) #include #include #include -#include -#include #include @@ -1953,10 +1951,10 @@ wavefront_download_firmware (snd_wavefront_t *dev, char *path) unsigned char section[WF_SECTION_MAX]; signed char section_length; /* yes, just a char; max value is WF_SECTION_MAX */ int section_cnt_downloaded = 0; - int fd; - int c; - int i; - mm_segment_t fs; + int c, i, ret = 0; + mm_segment_t fs = get_fs(); + struct file *filp; + loff_t pos; /* This tries to be a bit cleverer than the stuff Alan Cox did for the generic sound firmware, in that it actually knows @@ -1968,20 +1966,20 @@ wavefront_download_firmware (snd_wavefront_t *dev, char *path) 42 bytes (well, WF_SECTION_MAX) long. */ - fs = get_fs(); - set_fs (get_ds()); + set_fs(get_ds()); - if ((fd = sys_open ((char __user *) path, 0, 0)) < 0) { - snd_printk ("Unable to load \"%s\".\n", - path); + filp = filp_open(path, 0, 0); + if (IS_ERR(filp)) { + snd_printk("Unable to load \"%s\".\n", path); return 1; } while (1) { int x; - if ((x = sys_read (fd, (char __user *) §ion_length, sizeof (section_length))) != - sizeof (section_length)) { + pos = filp->f_pos; + x = vfs_read(filp, (char __user *) §ion_length, sizeof(section_length), &pos); + if (x != sizeof(section_length)) { snd_printk ("firmware read error.\n"); goto failure; } @@ -1996,9 +1994,10 @@ wavefront_download_firmware (snd_wavefront_t *dev, char *path) goto failure; } - if (sys_read (fd, (char __user *) section, section_length) != section_length) { - snd_printk ("firmware section " - "read error.\n"); + pos = filp->f_pos; + x = vfs_read(filp, (char __user *) section, section_length, &pos); + if (x != section_length) { + snd_printk ("firmware section read error.\n"); goto failure; } @@ -2034,19 +2033,17 @@ wavefront_download_firmware (snd_wavefront_t *dev, char *path) } } + goto success; - sys_close (fd); - set_fs (fs); - return 0; - - failure: - sys_close (fd); - set_fs (fs); +failure: + ret = 1; snd_printk ("firmware download failed!!!\n"); - return 1; +success: + filp_close(filp, current->files); + set_fs (fs); + return ret; } - static int __devinit wavefront_do_reset (snd_wavefront_t *dev) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kernel BUG at lib/list_debug.c:27!
Hi, > Hi, > > According to dmesg, I encountered a kernel bug on my system. > I'm not sure if this is the appropriate place to report this problem > as this occured on a Fedora kernel. Maybe its a general problem? Please file a bug at https://bugzilla.redhat.com/ under Product: Fedora, Version: f7, and Component: kernel. > kernel BUG at lib/list_debug.c:27! > invalid opcode: [2] SMP BZ#247975 looks similar, but I am not sure if it is the same issue. Hope this helps. Thanks, Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk->exit_state usage (resend)
tsk->exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk->exit_state & (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk->exit_state is sufficient. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk->exit_state & (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk->exit_state; const char **p = &task_state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk->exit_state); WARN_ON(atomic_read(&tsk->usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p->exit_state != EXIT_ZOMBIE && p->exit_state != EXIT_DEAD); + BUG_ON(!p->exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p->state == TASK_DEAD); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Make checkpatch rant about trailing ; at the end of "if" expr
Make checkpatch rant about trailing ; at the end of "if" expression. Thanks to Auke for the regexp. Signed-off by: Eugene Teo <[EMAIL PROTECTED]> --- checkpatch.pl-0.09.default 2007-08-03 23:31:40.0 +0800 +++ checkpatch.pl-0.09 2007-08-16 13:18:40.0 +0800 @@ -1091,6 +1091,12 @@ sub process { CHK("__setup appears un-documented -- check Documentation/kernel-parameters.txt\n" . $herecurr); } } + +# checks for trailing ; at the end of "if" expression + if ($line =~ /\bif\s*\([^\)]*\)\s*\;/) { + my $herevet = "$here\n" . cat_vet($line) . "\n"; + ERROR("trailing ;\n" . $herevet); + } } if ($chk_patch && !$is_patch) { - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Page Cache Question
> Adnan Khaleel wrote: >> I'm looking for a way to disable the page cache for an >> experimental NUMA system running the 2.6.17 kernel. I would prefer to >> only disable the page cache for my process and still have it be enabled >> by the rest of the system. Is there an easy way of doing this? >> Alternatively, I would be fine disabling the Page Cache altogether as >> well. >> > Assuming what you really mean is that you don't want to cache > file i/o for that process - try opening files with O_DIRECT. You will probably find this tool useful: http://userweb.kernel.org/~akpm/pagecache-management/ "... a little tool which permits the management of the pagecache usage of arbitrary applications. Effectively it prevents the targetted application from using any pagecache at all." Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Fix tsk->exit_state usage
tsk->exit_state can only be 0, EXIT_ZOMBIE, or EXIT_DEAD. A non-zero test is the same as tsk->exit_state & (EXIT_ZOMBIE | EXIT_DEAD), so just testing tsk->exit_state is sufficient. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/proc/array.c |3 +-- kernel/fork.c |2 +- kernel/sched.c |2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 965625a..babb24d 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -145,8 +145,7 @@ static inline const char *get_task_state(struct task_struct *tsk) TASK_UNINTERRUPTIBLE | TASK_STOPPED | TASK_TRACED)) | - (tsk->exit_state & (EXIT_ZOMBIE | - EXIT_DEAD)); + tsk->exit_state; const char **p = &task_state_array[0]; while (state) { diff --git a/kernel/fork.c b/kernel/fork.c index 7332e23..56d1b8b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -115,7 +115,7 @@ EXPORT_SYMBOL(free_task); void __put_task_struct(struct task_struct *tsk) { - WARN_ON(!(tsk->exit_state & (EXIT_DEAD | EXIT_ZOMBIE))); + WARN_ON(!tsk->exit_state); WARN_ON(atomic_read(&tsk->usage)); WARN_ON(tsk == current); diff --git a/kernel/sched.c b/kernel/sched.c index 45e17b8..8c27f08 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -5190,7 +5190,7 @@ static void migrate_dead(unsigned int dead_cpu, struct task_struct *p) struct rq *rq = cpu_rq(dead_cpu); /* Must be exiting, otherwise would be on tasklist. */ - BUG_ON(p->exit_state != EXIT_ZOMBIE && p->exit_state != EXIT_DEAD); + BUG_ON(!p->exit_state); /* Cannot have done final schedule yet: would have vanished. */ BUG_ON(p->state == TASK_DEAD); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers/scsi/ips.c: fix scsi_add_host warning
This patch fixes the following warning: drivers/scsi/ips.c: In function 'ips_register_scsi': drivers/scsi/ips.c:6867: warning: ignoring return value of 'scsi_add_host', declared with attribute warn_unused_result Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/scsi/ips.c | 16 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c index 492a51b..b04c42f 100644 --- a/drivers/scsi/ips.c +++ b/drivers/scsi/ips.c @@ -6824,13 +6824,14 @@ ips_order_controllers(void) static int ips_register_scsi(int index) { + int rc = -1; struct Scsi_Host *sh; ips_ha_t *ha, *oldha = ips_ha[index]; sh = scsi_host_alloc(&ips_driver_template, sizeof (ips_ha_t)); if (!sh) { IPS_PRINTK(KERN_WARNING, oldha->pcidev, "Unable to register controller with SCSI subsystem\n"); - return -1; + return rc; } ha = IPS_HA(sh); memcpy(ha, oldha, sizeof (ips_ha_t)); @@ -6839,8 +6840,7 @@ ips_register_scsi(int index) if (request_irq(ha->irq, do_ipsintr, IRQF_SHARED, ips_name, ha)) { IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to install interrupt handler\n"); - scsi_host_put(sh); - return -1; + goto err_put_sh; } kfree(oldha); @@ -6864,10 +6864,18 @@ ips_register_scsi(int index) sh->max_channel = ha->nbus - 1; sh->can_queue = ha->max_cmds - 1; - scsi_add_host(sh, NULL); + rc = scsi_add_host(sh, NULL); + if (rc) + goto err_free_irq; scsi_scan_host(sh); return 0; + +err_free_irq: + free_irq(ha->irq); +err_put_sh: + scsi_host_put(sh); + return rc; } /*---*/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ALSA] seq: resource leak fix and various code cleanups
> At Tue, 7 Aug 2007 18:52:49 +0800, > Eugene Teo wrote: [...] > I fixed these and applied your patch to ALSA tree now. > Thanks! Thanks! Will take note next time. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [ALSA] seq: resource leak fix and various code cleanups
Hi Takashi-san, > At Tue, 7 Aug 2007 16:40:48 +0800, > Eugene Teo wrote: > > > > This patch fixes: > > 1) a resource leak (CID: 1817) > > 2) various code cleanups [...] > > if (i >= SNDRV_SEQ_OSS_MAX_CLIENTS) { > > snd_printk(KERN_ERR "too many applications\n"); > > - kfree(dp); > > - return -ENOMEM; > > + rc = -ENOMEM; > > + goto _error; > > This seems wrong. It goes before initializing dp->queue = -1, so it > screws up delete_seq_queue(). You are right. The initialisations should come first. > > @@ -276,11 +281,13 @@ snd_seq_oss_open(struct file *file, int level) > > return 0; > > > > _error: > > - snd_seq_oss_synth_cleanup(dp); > > - snd_seq_oss_midi_cleanup(dp); > > - i = dp->queue; > > + snd_seq_oss_writeq_delete(dp->writeq); > > + snd_seq_oss_readq_delete(dp->readq); > > + delete_seq_queue(dp->queue); > > delete_port(dp); > > - delete_seq_queue(i); > > + snd_seq_oss_midi_cleanup(dp); > > + snd_seq_oss_synth_cleanup(dp); > > + kfree(dp); > > The order of these calls is important. Did you test it and confirm > that it has no side effects? Only snd_seq_oss_synth_cleanup() and snd_seq_oss_midi_cleanup() should be in order. The rest of the cleanup functions do not depend on one another. Thanks for your advice. Here's my second try: This patch fixes: 1) a resource leak (CID: 1817) 2) various code cleanups Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- sound/core/seq/oss/seq_oss_init.c | 40 +-- sound/core/seq/oss/seq_oss_writeq.c |6 +++- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c index ca5a2ed..f26b751 100644 --- a/sound/core/seq/oss/seq_oss_init.c +++ b/sound/core/seq/oss/seq_oss_init.c @@ -176,29 +176,31 @@ snd_seq_oss_open(struct file *file, int level) int i, rc; struct seq_oss_devinfo *dp; - if ((dp = kzalloc(sizeof(*dp), GFP_KERNEL)) == NULL) { + dp = kzalloc(sizeof(*dp), GFP_KERNEL); + if (!dp) { snd_printk(KERN_ERR "can't malloc device info\n"); return -ENOMEM; } debug_printk(("oss_open: dp = %p\n", dp)); + dp->cseq = system_client; + dp->port = -1; + dp->queue = -1; + dp->readq = NULL; + dp->writeq = NULL; + for (i = 0; i < SNDRV_SEQ_OSS_MAX_CLIENTS; i++) { if (client_table[i] == NULL) break; } + + dp->index = i; if (i >= SNDRV_SEQ_OSS_MAX_CLIENTS) { snd_printk(KERN_ERR "too many applications\n"); - kfree(dp); - return -ENOMEM; + rc = -ENOMEM; + goto _error; } - dp->index = i; - dp->cseq = system_client; - dp->port = -1; - dp->queue = -1; - dp->readq = NULL; - dp->writeq = NULL; - /* look up synth and midi devices */ snd_seq_oss_synth_setup(dp); snd_seq_oss_midi_setup(dp); @@ -211,14 +213,16 @@ snd_seq_oss_open(struct file *file, int level) /* create port */ debug_printk(("create new port\n")); - if ((rc = create_port(dp)) < 0) { + rc = create_port(dp); + if (rc < 0) { snd_printk(KERN_ERR "can't create port\n"); goto _error; } /* allocate queue */ debug_printk(("allocate queue\n")); - if ((rc = alloc_seq_queue(dp)) < 0) + rc = alloc_seq_queue(dp); + if (rc < 0) goto _error; /* set address */ @@ -235,7 +239,8 @@ snd_seq_oss_open(struct file *file, int level) /* initialize read queue */ debug_printk(("initialize read queue\n")); if (is_read_mode(dp->file_mode)) { - if ((dp->readq = snd_seq_oss_readq_new(dp, maxqlen)) == NULL) { + dp->readq = snd_seq_oss_readq_new(dp, maxqlen); + if (dp->readq == NULL) { rc = -ENOMEM; goto _error; } @@ -253,7 +258,8 @@ snd_seq_oss_open(struct file *file, int level) /* initialize timer */ debug_printk(("initialize timer\n")); - if ((dp->timer = snd_seq_oss_timer_new(dp)) == NULL) { + dp->timer = snd_seq_oss_timer_new(dp); + if (dp->timer == NULL) { snd_printk(KERN_ERR "can't alloc timer\n"); rc = -ENOMEM; goto _error; @@ -276,11 +282,13 @@ snd_seq_oss_open(struct file *file, int l
[ALSA] seq: resource leak fix and various code cleanups
This patch fixes: 1) a resource leak (CID: 1817) 2) various code cleanups Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- sound/core/seq/oss/seq_oss_init.c | 29 ++--- sound/core/seq/oss/seq_oss_writeq.c |6 -- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/sound/core/seq/oss/seq_oss_init.c b/sound/core/seq/oss/seq_oss_init.c index ca5a2ed..c9b95c3 100644 --- a/sound/core/seq/oss/seq_oss_init.c +++ b/sound/core/seq/oss/seq_oss_init.c @@ -176,7 +176,8 @@ snd_seq_oss_open(struct file *file, int level) int i, rc; struct seq_oss_devinfo *dp; - if ((dp = kzalloc(sizeof(*dp), GFP_KERNEL)) == NULL) { + dp = kzalloc(sizeof(*dp), GFP_KERNEL); + if (dp == NULL) { snd_printk(KERN_ERR "can't malloc device info\n"); return -ENOMEM; } @@ -188,8 +189,8 @@ snd_seq_oss_open(struct file *file, int level) } if (i >= SNDRV_SEQ_OSS_MAX_CLIENTS) { snd_printk(KERN_ERR "too many applications\n"); - kfree(dp); - return -ENOMEM; + rc = -ENOMEM; + goto _error; } dp->index = i; @@ -211,14 +212,16 @@ snd_seq_oss_open(struct file *file, int level) /* create port */ debug_printk(("create new port\n")); - if ((rc = create_port(dp)) < 0) { + rc = create_port(dp); + if (rc < 0) { snd_printk(KERN_ERR "can't create port\n"); goto _error; } /* allocate queue */ debug_printk(("allocate queue\n")); - if ((rc = alloc_seq_queue(dp)) < 0) + rc = alloc_seq_queue(dp); + if (rc < 0) goto _error; /* set address */ @@ -235,7 +238,8 @@ snd_seq_oss_open(struct file *file, int level) /* initialize read queue */ debug_printk(("initialize read queue\n")); if (is_read_mode(dp->file_mode)) { - if ((dp->readq = snd_seq_oss_readq_new(dp, maxqlen)) == NULL) { + dp->readq = snd_seq_oss_readq_new(dp, maxqlen); + if (dp->readq == NULL) { rc = -ENOMEM; goto _error; } @@ -253,7 +257,8 @@ snd_seq_oss_open(struct file *file, int level) /* initialize timer */ debug_printk(("initialize timer\n")); - if ((dp->timer = snd_seq_oss_timer_new(dp)) == NULL) { + dp->timer = snd_seq_oss_timer_new(dp); + if (dp->timer == NULL) { snd_printk(KERN_ERR "can't alloc timer\n"); rc = -ENOMEM; goto _error; @@ -276,11 +281,13 @@ snd_seq_oss_open(struct file *file, int level) return 0; _error: - snd_seq_oss_synth_cleanup(dp); - snd_seq_oss_midi_cleanup(dp); - i = dp->queue; + snd_seq_oss_writeq_delete(dp->writeq); + snd_seq_oss_readq_delete(dp->readq); + delete_seq_queue(dp->queue); delete_port(dp); - delete_seq_queue(i); + snd_seq_oss_midi_cleanup(dp); + snd_seq_oss_synth_cleanup(dp); + kfree(dp); return rc; } diff --git a/sound/core/seq/oss/seq_oss_writeq.c b/sound/core/seq/oss/seq_oss_writeq.c index 5c84956..2174248 100644 --- a/sound/core/seq/oss/seq_oss_writeq.c +++ b/sound/core/seq/oss/seq_oss_writeq.c @@ -63,8 +63,10 @@ snd_seq_oss_writeq_new(struct seq_oss_devinfo *dp, int maxlen) void snd_seq_oss_writeq_delete(struct seq_oss_writeq *q) { - snd_seq_oss_writeq_clear(q);/* to be sure */ - kfree(q); + if (q) { + snd_seq_oss_writeq_clear(q);/* to be sure */ + kfree(q); + } } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] drivers/video/geode/lxfb_core.c: cleanups
> This pacth contains the following cleanups: > - make the needlessly global geode_modedb[] static > - lxfb_setup(): remove an unused variable I have submitted a patch for the 2nd cleanup: http://marc.info/?l=linux-mm-commits&m=118616305111463&w=2 Thanks, Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/char/sonypi.c: fix ids member of struct acpi_driver
Hi Mattia, > On Thu, Aug 02, 2007 at 09:50:18AM +0200, Thomas Renninger wrote: > > On Thu, 2007-08-02 at 15:40 +0900, Mattia Dongili wrote: > > > On Wed, Aug 01, 2007 at 05:15:34PM +0800, Eugene Teo wrote: > > > > ids member of struct acpi_driver is of type struct acpi_device_id, not a > > > > character array. > > > > > > > > Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> [...] > yep, sony-laptop will replace sonypi. It still has some problems but > development is progressing in sony-laptop, not sonypi. > > Eugene, I'll import the patch without the MODULE_DEVICE_TABLE line. Ok, thanks. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers/scsi/advansys.c: fix advansys_board_found compile error
Hi Gabriel, Hope the following trivial patch helps. > Getting this with a randconfig ( http://194.231.229.228/MM/randconfig-auto-10 > ) > [...] > drivers/scsi/advansys.c:794:2: warning: #warning this driver is still not > properly converted to the DMA API > drivers/scsi/advansys.c: In function 'advansys_board_found': > drivers/scsi/advansys.c:17781: error: implicit declaration of function > 'to_pci_dev' [...] This patch fixes the following compile error: drivers/scsi/advansys.c: In function 'advansys_board_found': drivers/scsi/advansys.c:17781: error: implicit declaration of function 'to_pci_dev' Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/scsi/advansys.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c index 79c0b6e..908f02b 100644 --- a/drivers/scsi/advansys.c +++ b/drivers/scsi/advansys.c @@ -774,6 +774,7 @@ #include #include #include +#include #include #include - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers/video/geode/lxfb_core.c: fix lxfb_setup warning
This patch fixes the following warning: drivers/video/geode/lxfb_core.c: In function 'lxfb_setup': drivers/video/geode/lxfb_core.c:564: warning: unused variable 'opt' Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/video/geode/lxfb_core.c |7 +-- 1 files changed, 1 insertions(+), 6 deletions(-) diff --git a/drivers/video/geode/lxfb_core.c b/drivers/video/geode/lxfb_core.c index 5e30b40..255c8b3 100644 --- a/drivers/video/geode/lxfb_core.c +++ b/drivers/video/geode/lxfb_core.c @@ -566,12 +566,7 @@ static int __init lxfb_setup(char *options) if (!options || !*options) return 0; - while (1) { - char *opt = strsep(&options, ","); - - if (opt == NULL) - break; - + while ( (opt = strsep(&options, ",")) != NULL) { if (!*opt) continue; - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers/char/sonypi.c: fix ids member of struct acpi_driver
ids member of struct acpi_driver is of type struct acpi_device_id, not a character array. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/char/sonypi.c |8 +++- 1 files changed, 7 insertions(+), 1 deletions(-) diff --git a/drivers/char/sonypi.c b/drivers/char/sonypi.c index 73037a4..ac0aeb0 100644 --- a/drivers/char/sonypi.c +++ b/drivers/char/sonypi.c @@ -1147,10 +1147,16 @@ static int sonypi_acpi_remove(struct acpi_device *device, int type) return 0; } +const static struct acpi_device_id sonypi_device_ids[] = { + {"SNY6001", 0}, + {"", 0}, +}; +MODULE_DEVICE_TABLE(acpi, sonypi_device_ids); + static struct acpi_driver sonypi_acpi_driver = { .name = "sonypi", .class = "hkey", - .ids= "SNY6001", + .ids= sonypi_device_ids, .ops= { .add = sonypi_acpi_add, .remove = sonypi_acpi_remove, - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] - Remove current defines and uses of pr_err, add pr_emerg, pr_alert, pr_crit, pr_err, pr_warn, pr_notice to include/linux/kernel.h
Hi Joe, Joe Perches wrote: > Remove current #define and uses of pr_err > Add pr_emerg, pr_alert, pr_crit, pr_err, pr_warn, pr_notice > to include/linux/kernel.h > > Signed-off-by: Joe Perches <[EMAIL PROTECTED]> > > diff --git a/drivers/i2c/chips/menelaus.c b/drivers/i2c/chips/menelaus.c > index 48a7e2f..3ee323a 100644 > --- a/drivers/i2c/chips/menelaus.c > +++ b/drivers/i2c/chips/menelaus.c > @@ -49,8 +49,7 @@ > #include > > #define DRIVER_NAME "menelaus" > - > -#define pr_err(fmt, arg...) printk(KERN_ERR DRIVER_NAME ": ", ## arg); Makes sense to remove the duplicated pr_* functions, and have it declared in include/linux/kernel.h. > +#define PFX DRIVER_NAME ": " > > #define MENELAUS_I2C_ADDRESS 0x72 > > @@ -156,7 +155,7 @@ static int menelaus_write_reg(int reg, u8 value) > int val = i2c_smbus_write_byte_data(the_menelaus->client, reg, value); > > if (val < 0) { > - pr_err("write error"); > + printk(KERN_ERR PFX "write error\n"); > return val; > } [...] But why are you replacing the existing pr_*() with printk(KERN_*? Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] coredump: cleanup documentation for suid_dumpable
Hi Alan, Alan Cox wrote: > On Tue, 31 Jul 2007 15:03:40 +0800 > Eugene Teo <[EMAIL PROTECTED]> wrote: > >> This patch removes documentation that is related to suidsafe core dump >> mode. >> >> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> > > NAK - this feature is actively used and can be set by the sysctl > interface. The PRCTL fixup was just a bug being fixed. The sysctl > interface is still relevant. Thank you for correcting me. Appreciated. I am not aware that it is still being actively used. May I know who or which program is using this feature? Thanks, Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 3/3] coredump: re-implement suid_dumpable using a flag
Hidehiro-san re-implemented suid_dumpable using a pair of bit flags. But since we no longer permitting users to call prctl(PR_SET_DUMPABLE, 2), there is no need to waste a bit of mm_struct.flags for something that will never be used. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/exec.c | 42 +- include/linux/sched.h | 13 ++--- 2 files changed, 11 insertions(+), 44 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 60b4080..0f30b94 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1666,53 +1666,21 @@ fail: } /* - * set_dumpable converts traditional three-value dumpable to two flags and - * stores them into mm->flags. It modifies lower two bits of mm->flags, but - * these bits are not changed atomically. So get_dumpable can observe the - * intermediate state. To avoid doing unexpected behavior, get get_dumpable - * return either old dumpable or new one by paying attention to the order of - * modifying the bits. - * - * dumpable | mm->flags (binary) - * old new | initial interim final - * -+--- - * 01 | 00 01 01 - * 02 | 00 10(*) 11 - * 10 | 01 00 00 - * 12 | 01 11 11 - * 20 | 11 10(*) 00 - * 21 | 11 11 01 - * - * (*) get_dumpable regards interim value of 10 as 11. + * set_dumpable converts traditional two-value dumpable to one flag and + * stores it in mm->flags. It modifies the lower bit of mm->flags. */ void set_dumpable(struct mm_struct *mm, int value) { - switch (value) { - case 0: + if (value == 0) clear_bit(MMF_DUMPABLE, &mm->flags); - smp_wmb(); - clear_bit(MMF_DUMP_SECURELY, &mm->flags); - break; - case 1: - set_bit(MMF_DUMPABLE, &mm->flags); - smp_wmb(); - clear_bit(MMF_DUMP_SECURELY, &mm->flags); - break; - case 2: - set_bit(MMF_DUMP_SECURELY, &mm->flags); - smp_wmb(); + else if (value == 1) set_bit(MMF_DUMPABLE, &mm->flags); - break; - } } EXPORT_SYMBOL_GPL(set_dumpable); int get_dumpable(struct mm_struct *mm) { - int ret; - - ret = mm->flags & 0x3; - return (ret >= 2) ? 2 : ret; + return (mm->flags & 0x1); } int do_coredump(long signr, int exit_code, struct pt_regs * regs) diff --git a/include/linux/sched.h b/include/linux/sched.h index 2e49027..8a0092d 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -350,15 +350,14 @@ extern int get_dumpable(struct mm_struct *mm); /* mm flags */ /* dumpable bits */ -#define MMF_DUMPABLE 0 /* core dump is permitted */ -#define MMF_DUMP_SECURELY 1 /* core file is readable only by root */ -#define MMF_DUMPABLE_BITS 2 +#define MMF_DUMPABLE 0 /* core dump is permitted */ +#define MMF_DUMPABLE_BITS 1 /* coredump filter bits */ -#define MMF_DUMP_ANON_PRIVATE 2 -#define MMF_DUMP_ANON_SHARED 3 -#define MMF_DUMP_MAPPED_PRIVATE4 -#define MMF_DUMP_MAPPED_SHARED 5 +#define MMF_DUMP_ANON_PRIVATE 1 +#define MMF_DUMP_ANON_SHARED 2 +#define MMF_DUMP_MAPPED_PRIVATE3 +#define MMF_DUMP_MAPPED_SHARED 4 #define MMF_DUMP_FILTER_SHIFT MMF_DUMPABLE_BITS #define MMF_DUMP_FILTER_BITS 4 #define MMF_DUMP_FILTER_MASK \ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/3] coredump: remove suidsafe mode related dead code
This patch removes suidsafe core dump mode related dead code. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/exec.c | 16 +--- include/linux/binfmts.h |3 --- 2 files changed, 1 insertions(+), 18 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index 7bdea79..60b4080 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1723,8 +1723,6 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs) struct inode * inode; struct file * file; int retval = 0; - int fsuid = current->fsuid; - int flag = 0; int ispipe = 0; audit_core_dumps(signr); @@ -1737,16 +1735,6 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs) up_write(&mm->mmap_sem); goto fail; } - - /* -* We cannot trust fsuid as being the "true" uid of the -* process nor do we know its entire history. We only know it -* was tainted so we dump it as root in mode 2. -*/ - if (get_dumpable(mm) == 2) {/* Setuid core dump mode */ - flag = O_EXCL; /* Stop rewrite attacks */ - current->fsuid = 0; /* Dump root private */ - } set_dumpable(mm, 0); retval = coredump_wait(exit_code); @@ -1778,8 +1766,7 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs) } } else file = filp_open(corename, -O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE | flag, -0600); +O_CREAT | 2 | O_NOFOLLOW | O_LARGEFILE, 0600); if (IS_ERR(file)) goto fail_unlock; inode = file->f_path.dentry->d_inode; @@ -1806,7 +1793,6 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs) close_fail: filp_close(file, NULL); fail_unlock: - current->fsuid = fsuid; complete_all(&mm->core_done); fail: return retval; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 91c8c07..ca75ee4 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -81,9 +81,6 @@ extern int search_binary_handler(struct linux_binprm *,struct pt_regs *); extern int flush_old_exec(struct linux_binprm * bprm); extern int suid_dumpable; -#define SUID_DUMP_DISABLE 0 /* No setuid dumping */ -#define SUID_DUMP_USER 1 /* Dump as user of process */ -#define SUID_DUMP_ROOT 2 /* Dump as root */ /* Stack area protections */ #define EXSTACK_DEFAULT 0/* Whatever the arch defaults to */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/3] coredump: cleanup documentation for suid_dumpable
This patch removes documentation that is related to suidsafe core dump mode. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- Documentation/sysctl/fs.txt |6 -- 1 files changed, 0 insertions(+), 6 deletions(-) diff --git a/Documentation/sysctl/fs.txt b/Documentation/sysctl/fs.txt index aa986a3..2318f7a 100644 --- a/Documentation/sysctl/fs.txt +++ b/Documentation/sysctl/fs.txt @@ -142,12 +142,6 @@ or otherwise protected/tainted binaries. The modes are 1 - (debug) - all processes dump core when possible. The core dump is owned by the current user and no security is applied. This is intended for system debugging situations only. Ptrace is unchecked. -2 - (suidsafe) - any binary which normally would not be dumped is dumped - readable by root only. This allows the end user to remove - such a dump but not access it directly. For security reasons - core dumps in this mode will not overwrite one another or - other files. This mode is appropriate when administrators are - attempting to debug problems in a normal environment. == - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] coredump: setuid core dump cleanups
Hi, A year ago, commit abf75a5033d4da7b8a7e92321d74021d1fcfb502 was included to fix a security vulnerability that is related to prctl privilege escalation, and suid_dumpable (CVE-2006-2451). But the commit was just a quick fix to prevent users from calling prctl(PR_SET_DUMPABLE, 2). This patch series try to remove code that is related to the value 2 (suidsafe) core dump mode, and also re-implement Hidehiro-san's re-implementation of dumpable using a bit flag instead of a pair (see commit 6c5d523826dc639df709ed0f88c5d2ce25379652). Thanks, Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/partitions/check.c: add_partition() warning fixes (take 4)
This patch fixes these warnings: fs/partitions/check.c: In function 'add_partition': fs/partitions/check.c:391: warning: ignoring return value of 'kobject_add', declared with attribute warn_unused_result fs/partitions/check.c:394: warning: ignoring return value of 'sysfs_create_link', declared with attribute warn_unused_result fs/partitions/check.c:401: warning: ignoring return value of 'sysfs_create_file', declared with attribute warn_unused_result Got it right this time. Thanks Cornelia for help. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/partitions/check.c | 23 --- 1 files changed, 20 insertions(+), 3 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 783c57e..bc69f81 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -371,6 +371,7 @@ void delete_partition(struct gendisk *disk, int part) void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags) { struct hd_struct *p; + int err; p = kzalloc(sizeof(*p), GFP_KERNEL); if (!p) @@ -388,20 +389,36 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, p->kobj.parent = &disk->kobj; p->kobj.ktype = &ktype_part; kobject_init(&p->kobj); - kobject_add(&p->kobj); + err = kobject_add(&p->kobj); + if (err) + goto err_out; if (!disk->part_uevent_suppress) kobject_uevent(&p->kobj, KOBJ_ADD); - sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + err = sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + if (err) + goto err_out_del_kobj; if (flags & ADDPART_FLAG_WHOLEDISK) { static struct attribute addpartattr = { .name = "whole_disk", .mode = S_IRUSR | S_IRGRP | S_IROTH, }; - sysfs_create_file(&p->kobj, &addpartattr); + err = sysfs_create_file(&p->kobj, &addpartattr); + if (err) + goto err_out_del_link; } partition_sysfs_add_subdir(p); disk->part[part-1] = p; + return; + +err_out_del_link: + sysfs_remove_link(&p->kobj, "subsystem"); +err_out_del_kobj: + if (!disk->part_uevent_suppress) + kobject_uevent(&p->kobj, KOBJ_REMOVE); + kobject_del(&p->kobj); +err_out: + kobject_put(&p->kobj); } static char *make_block_name(struct gendisk *disk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/partitions/check.c: add_partition() warning fixes (take 3)
This patch fixes these warnings: fs/partitions/check.c: In function 'add_partition': fs/partitions/check.c:391: warning: ignoring return value of 'kobject_add', declared with attribute warn_unused_result fs/partitions/check.c:394: warning: ignoring return value of 'sysfs_create_link', declared with attribute warn_unused_result fs/partitions/check.c:401: warning: ignoring return value of 'sysfs_create_file', declared with attribute warn_unused_result Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/partitions/check.c | 22 +++--- 1 files changed, 19 insertions(+), 3 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 783c57e..a77b5dd 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -371,6 +371,7 @@ void delete_partition(struct gendisk *disk, int part) void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags) { struct hd_struct *p; + int err; p = kzalloc(sizeof(*p), GFP_KERNEL); if (!p) @@ -388,20 +389,35 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, p->kobj.parent = &disk->kobj; p->kobj.ktype = &ktype_part; kobject_init(&p->kobj); - kobject_add(&p->kobj); + err = kobject_add(&p->kobj); + if (err) + goto err_out; if (!disk->part_uevent_suppress) kobject_uevent(&p->kobj, KOBJ_ADD); - sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + err = sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + if (err) + goto err_out_del_uevent; if (flags & ADDPART_FLAG_WHOLEDISK) { static struct attribute addpartattr = { .name = "whole_disk", .mode = S_IRUSR | S_IRGRP | S_IROTH, }; - sysfs_create_file(&p->kobj, &addpartattr); + err = sysfs_create_file(&p->kobj, &addpartattr); + if (err) + goto err_out_del_link; } partition_sysfs_add_subdir(p); disk->part[part-1] = p; + return; + +err_out_del_link: + sysfs_remove_link(&p->kobj, "subsystem"); +err_out_del_uevent: + if (!disk->part_uevent_suppress) + kobject_uevent(&p->kobj, KOBJ_REMOVE); +err_out: + kobject_put(&p->kobj); } static char *make_block_name(struct gendisk *disk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/partitions/check.c: add_partition() warning fixes (take 2)
Cornelia Huck wrote: > On Mon, 30 Jul 2007 17:47:55 +0800, > Eugene Teo <[EMAIL PROTECTED]> wrote: > >> +err_out_del_link: >> +sysfs_remove_link(&p->kobj, "subsystem"); >> +err_out_del_kobj: >> +if (!disk->part_uevent_suppress) >> +kobject_uevent(&p->kobj, KOBJ_REMOVE); >> +kobject_put(&p->kobj); >> +err_out: >> +kfree(p); >> } > > No, this is wrong. You need to move the put behind err_out and remove > the kfree. (The release function will take care of p.) 503 void kobject_put(struct kobject * kobj) 504 { 505 if (kobj) 506 kref_put(&kobj->kref, kobject_release); [...] 52 int kref_put(struct kref *kref, void (*release)(struct kref *kref)) 53 { [...] 57 if (atomic_dec_and_test(&kref->refcount)) { 58 release(kref); 59 return 1; Nod, thanks for your guidance. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/partitions/check.c: add_partition() warning fixes (take 2)
This patch fixes these warnings: fs/partitions/check.c: In function 'add_partition': fs/partitions/check.c:391: warning: ignoring return value of 'kobject_add', declared with attribute warn_unused_result fs/partitions/check.c:394: warning: ignoring return value of 'sysfs_create_link', declared with attribute warn_unused_result fs/partitions/check.c:401: warning: ignoring return value of 'sysfs_create_file', declared with attribute warn_unused_result Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/partitions/check.c | 23 --- 1 files changed, 20 insertions(+), 3 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 783c57e..57cc590 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -371,6 +371,7 @@ void delete_partition(struct gendisk *disk, int part) void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags) { struct hd_struct *p; + int err; p = kzalloc(sizeof(*p), GFP_KERNEL); if (!p) @@ -388,20 +389,36 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, p->kobj.parent = &disk->kobj; p->kobj.ktype = &ktype_part; kobject_init(&p->kobj); - kobject_add(&p->kobj); + err = kobject_add(&p->kobj); + if (err) + goto err_out; if (!disk->part_uevent_suppress) kobject_uevent(&p->kobj, KOBJ_ADD); - sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + err = sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + if (err) + goto err_out_del_kobj; if (flags & ADDPART_FLAG_WHOLEDISK) { static struct attribute addpartattr = { .name = "whole_disk", .mode = S_IRUSR | S_IRGRP | S_IROTH, }; - sysfs_create_file(&p->kobj, &addpartattr); + err = sysfs_create_file(&p->kobj, &addpartattr); + if (err) + goto err_out_del_link; } partition_sysfs_add_subdir(p); disk->part[part-1] = p; + return; + +err_out_del_link: + sysfs_remove_link(&p->kobj, "subsystem"); +err_out_del_kobj: + if (!disk->part_uevent_suppress) + kobject_uevent(&p->kobj, KOBJ_REMOVE); + kobject_put(&p->kobj); +err_out: + kfree(p); } static char *make_block_name(struct gendisk *disk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/partitions/check.c: add_partition() warning fixes
Hi Cornelia, Cornelia Huck wrote: > On Sun, 29 Jul 2007 10:53:39 +0800, > Eugene Teo <[EMAIL PROTECTED]> wrote: [...] >> +return; >> + >> +err_out_del_link: >> +sysfs_remove_link(&p->kobj, "subsystem"); > > You need a remove uevent if you did an add uevent above. > >> +err_out_del_kobj: >> +kobject_del(&p->kobj); >> +err_out: >> +kfree(p); > > Please use kobject_put() instead. Thanks for your feedback. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/bluetooth/hci_ldisc.c: fix possible NULL dereferences
Hi Marcel, Marcel Holtmann wrote: Commit 22ad42033b7d2b3d7928fba9f89d1c7f8a3c9581 did not completely fix all the possible NULL dereferences. Besides hci_uart_close(), we also need to make sure that hdev is valid before calling hci_{unregister,free}_dev(). >>> I don't see any issue. Without HCI_UART_PROTO_SET, the hdev will never >>> be registered. So no need to protect it twice. >> Correct me if I am wrong. HCI_UART_PROTO_SET bit is only set if >> hci_uart_tty_ioctl() >> is called with HCIUARTSETPROTO. Is it possible for the HCI device to be >> registered >> and then unregistered without setting the HCI_UART_PROTO_SET bit in >> hdev->flags? > > look at the code. The hci_uart_tty_ioctl() is the only function that can > register the HCI device. So besides opening the TTY and set the line > discipline, you also have to the set the UART protocol running on top. I > don't see any way you can achieve to register a HCI device without > setting the HCI_UART_PROTO_SET bit in hu->flags. Ok. Thanks for the explanation. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] core_pattern: cleaned up repost/continuing post of core_pattern enhancements
Hi Martin, Martin Pitt wrote: > Eugene Teo [2007-07-29 21:03 +0800]: >>>> Also, it is probably good to think how we can "drop privileges" while >>>> piping >>>> the core dump output to an external program. A malicious user can >>>> potentially >>>> use it as a possible backdoor since anything that is executed by >>>> "|program" will >>>> be executed with root privileges. >>>> >>> It was my understanding that apport already did this. >> I haven't looked at apport yet, but are you talking about the userspace >> portion of >> apport or the kernel changes in the Ubuntu kernel? > > Similarly to Neil's patches, the Ubuntu kernel calls the userspace > helper as root, too. Apport drops privileges to the target process as > soon as possible (there are a few things it needs to do before, like > opening an fd to the crash file in /var/crash/ if that is only > writeable by root). Just sharing some thoughts. Wouldn't it be more logical to drop the privileges first, then call the userspace helper program? I know that this will limit tools like apport to be able to read and/or write files that are only writable by root, but there ought to be a better way to do this? What if the program piped is not a legitimate program? Also, maybe it is good to make this portion of the code optional too, so that if no one is using this "ispipe" feature, we just turn it off. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/bluetooth/hci_ldisc.c: fix possible NULL dereferences
Hi Marcel, Marcel Holtmann wrote: >> Commit 22ad42033b7d2b3d7928fba9f89d1c7f8a3c9581 did not completely fix all >> the possible NULL dereferences. Besides hci_uart_close(), we also need to >> make sure that hdev is valid before calling hci_{unregister,free}_dev(). > > I don't see any issue. Without HCI_UART_PROTO_SET, the hdev will never > be registered. So no need to protect it twice. Correct me if I am wrong. HCI_UART_PROTO_SET bit is only set if hci_uart_tty_ioctl() is called with HCIUARTSETPROTO. Is it possible for the HCI device to be registered and then unregistered without setting the HCI_UART_PROTO_SET bit in hdev->flags? Thanks, Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] drivers/bluetooth/hci_ldisc.c: fix possible NULL dereferences
Commit 22ad42033b7d2b3d7928fba9f89d1c7f8a3c9581 did not completely fix all the possible NULL dereferences. Besides hci_uart_close(), we also need to make sure that hdev is valid before calling hci_{unregister,free}_dev(). Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- drivers/bluetooth/hci_ldisc.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 6055b9c..4813f7c 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -308,11 +308,10 @@ static void hci_uart_tty_close(struct tty_struct *tty) if (hu) { struct hci_dev *hdev = hu->hdev; - if (hdev) + if (hdev) { hci_uart_close(hdev); - - if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) { - hu->proto->close(hu); + if (test_and_clear_bit(HCI_UART_PROTO_SET, &hu->flags)) + hu->proto->close(hu); hci_unregister_dev(hdev); hci_free_dev(hdev); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/3] core_pattern: fix up a few miscelaneous bugs
Neil Horman wrote: [...] > + delimit = strrchr(helper_argv[0], '/'); > + if (delimit) Trailing space. > + delimit++; > + else > + delimit = helper_argv[0]; > + if (!strcmp(delimit, current->comm)) > + { Trailing space, and '{' should be after '...comm))'. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/3] core_pattern: allow passing of arguments to user mode helper when core_pattern is a pipe
Neil Horman wrote: [...] > + /* core limit size */ > + case 'c': > + rc = snprintf(out_ptr, out_end - out_ptr, > + "%lu", > current->signal->rlim[RLIMIT_CORE].rlim_cur); Trailing space. [...] > - if(call_usermodehelper_pipe(corename+1, NULL, NULL, &file)) { > + if(call_usermodehelper_pipe(corename+1, helper_argv, NULL, > &file)) { ^ Use tabs, and a missing space after '('. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] core_pattern: ignore RLIMIT_CORE if core_pattern is a pipe
Neil Horman wrote: [...] > + * Don't bother to check the RLIMIT_CORE value if core_pattern points > + * to a pipe. Since we're not writing directly to the filesystem > + * RLIMIT_CORE doesn't really apply, as no actual core file will be > + * created unless the pipe reader choses to write out the core file > + * at which point file size limits and permissions will be imposed ^^^ Trailing space. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] core_pattern: cleaned up repost/continuing post of core_pattern enhancements
Neil Horman wrote: > On Sun, Jul 29, 2007 at 06:40:43PM +0800, Eugene Teo wrote: >> Neil Horman wrote: [...] >> You may want to improve your patches with style-related changes, including >> removing trailing spaces, using tabs instead of spaces, and defining pointers >> like char *ptr instead of char * ptr. >> > I assume this is just a general comment, since as far as I can see, I've > followed those guidelines. Please see the next few emails. >> Also, it is probably good to think how we can "drop privileges" while piping >> the core dump output to an external program. A malicious user can potentially >> use it as a possible backdoor since anything that is executed by "|program" >> will >> be executed with root privileges. >> > It was my understanding that apport already did this. I haven't looked at apport yet, but are you talking about the userspace portion of apport or the kernel changes in the Ubuntu kernel? Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] core_pattern: cleaned up repost/continuing post of core_pattern enhancements
Neil Horman wrote: > Ok, here we go > > As promised, I'm reposting the core_pattern enhancements I've done over the > past > few days. These three patches replace and conintue the work contained in the > following patches, and can replace them: > update-coredump-path-in-kernel-to-not-check-coredump-rlim-if-core_pattern-is-a-pipe.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe-fix.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe-fix-2.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe-fix-2-fix.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe-sparc64-fix.patch > allow-individual-core-dump-methods-to-be-unlimited-when-sending-to-a-pipe-fix-2-sparc64-fix.patch [...] You may want to improve your patches with style-related changes, including removing trailing spaces, using tabs instead of spaces, and defining pointers like char *ptr instead of char * ptr. Also, it is probably good to think how we can "drop privileges" while piping the core dump output to an external program. A malicious user can potentially use it as a possible backdoor since anything that is executed by "|program" will be executed with root privileges. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/partitions/check.c: add_partition() warning fixes
This patch fixes these warnings: fs/partitions/check.c: In function ‘add_partition’: fs/partitions/check.c:391: warning: ignoring return value of ‘kobject_add’, declared with attribute warn_unused_result fs/partitions/check.c:394: warning: ignoring return value of ‘sysfs_create_link’, declared with attribute warn_unused_result fs/partitions/check.c:401: warning: ignoring return value of ‘sysfs_create_file’, declared with attribute warn_unused_result Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/partitions/check.c | 21 ++--- 1 files changed, 18 insertions(+), 3 deletions(-) diff --git a/fs/partitions/check.c b/fs/partitions/check.c index 783c57e..01397c1 100644 --- a/fs/partitions/check.c +++ b/fs/partitions/check.c @@ -371,6 +371,7 @@ void delete_partition(struct gendisk *disk, int part) void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, int flags) { struct hd_struct *p; + int err; p = kzalloc(sizeof(*p), GFP_KERNEL); if (!p) @@ -388,20 +389,34 @@ void add_partition(struct gendisk *disk, int part, sector_t start, sector_t len, p->kobj.parent = &disk->kobj; p->kobj.ktype = &ktype_part; kobject_init(&p->kobj); - kobject_add(&p->kobj); + err = kobject_add(&p->kobj); + if (err) + goto err_out; if (!disk->part_uevent_suppress) kobject_uevent(&p->kobj, KOBJ_ADD); - sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + err = sysfs_create_link(&p->kobj, &block_subsys.kobj, "subsystem"); + if (err) + goto err_out_del_kobj; if (flags & ADDPART_FLAG_WHOLEDISK) { static struct attribute addpartattr = { .name = "whole_disk", .mode = S_IRUSR | S_IRGRP | S_IROTH, }; - sysfs_create_file(&p->kobj, &addpartattr); + err = sysfs_create_file(&p->kobj, &addpartattr); + if (err) + goto err_out_del_link; } partition_sysfs_add_subdir(p); disk->part[part-1] = p; + return; + +err_out_del_link: + sysfs_remove_link(&p->kobj, "subsystem"); +err_out_del_kobj: + kobject_del(&p->kobj); +err_out: + kfree(p); } static char *make_block_name(struct gendisk *disk) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] arch/i386/kernel/apm.c: apm_init() warning fix
arch/i386/kernel/apm.c: In function 'apm_init': arch/i386/kernel/apm.c:2240: warning: format '%lx' expects type 'long unsigned int', but argument 3 has type 'u32' apm_info.bios.offset is of type 'u32'. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- arch/i386/kernel/apm.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c index 47001d5..f02a8ac 100644 --- a/arch/i386/kernel/apm.c +++ b/arch/i386/kernel/apm.c @@ -2235,7 +2235,7 @@ static int __init apm_init(void) apm_info.bios.cseg_16_len = 0; /* 64k */ if (debug) { - printk(KERN_INFO "apm: entry %x:%lx cseg16 %x dseg %x", + printk(KERN_INFO "apm: entry %x:%x cseg16 %x dseg %x", apm_info.bios.cseg, apm_info.bios.offset, apm_info.bios.cseg_16, apm_info.bios.dseg); if (apm_info.bios.version > 0x100) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/afs/flock.c: posix_test_lock() returns void
posix_test_lock() returns void, so there is no need to test the return value. Checking fl->fl_type for F_UNLCK instead. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- fs/afs/flock.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/fs/afs/flock.c b/fs/afs/flock.c index 8f07f8d..4f77f3c 100644 --- a/fs/afs/flock.c +++ b/fs/afs/flock.c @@ -456,7 +456,8 @@ static int afs_do_getlk(struct file *file, struct file_lock *fl) /* check local lock records first */ ret = 0; - if (posix_test_lock(file, fl) == 0) { + posix_test_lock(file, fl); + if (fl->fl_type == F_UNLCK) { /* no local locks; consult the server */ ret = afs_vnode_fetch_status(vnode, NULL, key); if (ret < 0) -- 1.5.2.2 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Chinese translation of Documentation/HOWTO
> This is a Chinese translated version of Documentation/HOWTO. Currently > Chinese involvement in Linux kernel is very low, especially comparing to > its largest population base. Language could be the main obstacle. Hope > this document will help more Chinese to contribute to Linux kernel. > > Signed-off-by: Li Yang <[EMAIL PROTECTED]> > Signed-off-by: TripleX Chung <[EMAIL PROTECTED]> > Signed-off-by: Maggie Chen <[EMAIL PROTECTED]> > --- > Address comments from Wang Cong. > Documentation/zh_CN/HOWTO | 536 > + > 1 files changed, 536 insertions(+), 0 deletions(-) > create mode 100644 Documentation/zh_CN/HOWTO > > diff --git a/Documentation/zh_CN/HOWTO b/Documentation/zh_CN/HOWTO > new file mode 100644 > index 000..c70debd > --- /dev/null > +++ b/Documentation/zh_CN/HOWTO [...] I read through the translated HOWTO. Looks good to me. Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] drivers/net/wireless/libertas/rx.c: fix use-after-free
> On Sat, May 19, 2007 at 11:14:10AM +0800, Eugene Teo wrote: > > John W. Linville wrote: > > > >> done: > > >> LEAVE(); > > >> > > >> - skb->protocol = __constant_htons(0x0019); /* > > >> ETH_P_80211_RAW */ > > >> - > > > > > > Except for this part...is this intentional? > > > > skb could have been freed by then. And, in libertas_upload_rx_packet(), > > skb->protocol > > is initialized by eth_type_trans(skb, priv->wlan_dev.netdev). > > OK, I see that. Looks like Florin has reposted his patch, still > without this hunk. Would you like to submit a patch for this hunk? skb could have been freed by then. Also, in libertas_upload_rx_packet(), skb->protocol is initialized by eth_type_trans(). Cc: John W. Linville <[EMAIL PROTECTED]> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> --- diff -uprN -X 2.6.22-rc2/Documentation/dontdiff 2.6.22-rc2.default/drivers/net/wireless/libertas/rx.c 2.6.22-rc2/drivers/net/wireless/libertas/rx.c --- 2.6.22-rc2.default/drivers/net/wireless/libertas/rx.c 2007-05-21 22:07:50.0 +0800 +++ 2.6.22-rc2/drivers/net/wireless/libertas/rx.c 2007-05-21 22:08:44.0 +0800 @@ -453,7 +453,5 @@ static int process_rxed_802_11_packet(wl done: LEAVE(); - skb->protocol = __constant_htons(0x0019); /* ETH_P_80211_RAW */ - return (ret); } -- 1024D/58DF8823 print 47B9 90F6 AE4A 9C51 37E0 D6E1 EA84 C6A2 58DF 8823 main(i) { putchar(182623909 >> (i-1) * 5&31|!!(i<7)<<6) && main(++i); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] net/llc/llc_conn.c: fix possible NULL dereference
Hi Randy, Randy Dunlap wrote: > On Sat, 19 May 2007 13:13:07 +0800 Eugene Teo wrote: > >> skb_peek() might return an empty list. skb should be checked before calling >> llc_pdu_sn_hdr() with it. >> >> Spotted by the Coverity checker. >> >> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> [...] > > Networking patches need to be sent to the [EMAIL PROTECTED] > mailing list (and lkml can be omitted IMHO). > > But... instead of doing the assignment and test in one swoop, > we prefer: Right, thanks for the reminder! Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.6 patch] net/llc/llc_conn.c: fix possible NULL dereference
skb_peek() might return an empty list. skb should be checked before calling llc_pdu_sn_hdr() with it. Spotted by the Coverity checker. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> diff --git a/net/llc/llc_conn.c b/net/llc/llc_conn.c index 3b8cfbe..28a3994 100644 --- a/net/llc/llc_conn.c +++ b/net/llc/llc_conn.c @@ -323,7 +323,8 @@ int llc_conn_remove_acked_pdus(struct sock *sk, u8 nr, u16 *how_many_unacked) if (!q_len) goto out; - skb = skb_peek(&llc->pdu_unack_q); + if (! (skb = skb_peek(&llc->pdu_unack_q))) + goto out; pdu = llc_pdu_sn_hdr(skb); /* finding position of last acked pdu in queue */ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] drivers/net/wireless/libertas/rx.c: fix use-after-free
John W. Linville wrote: > First, please send all wireless patches to > [EMAIL PROTECTED], and be sure to CC me as well...thanks! > > On Sat, May 19, 2007 at 12:50:31AM +0800, Eugene Teo wrote: >> libertas_upload_rx_packet() calls netif_rx() before returning, and it always >> return 0. >> Also within libertas_upload_rx_packet(), it will initialize skb->protocol >> anyways. >> >> Spotted by the Coverity checker. > > A nearly identical patch was posted by Florin Malita <[EMAIL PROTECTED]> > to netdev (also the wrong list) on Wednesday evening. Nod. I wasn't subscribed to netdev list. >> done: >> LEAVE(); >> >> - skb->protocol = __constant_htons(0x0019); /* ETH_P_80211_RAW */ >> - > > Except for this part...is this intentional? skb could have been freed by then. And, in libertas_upload_rx_packet(), skb->protocol is initialized by eth_type_trans(skb, priv->wlan_dev.netdev). Eugene - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] drivers/net/wireless/libertas/fw.c: fix use-before-check
Hi John, John W. Linville wrote: > On Sat, May 19, 2007 at 01:06:49AM +0800, Eugene Teo wrote: >> NULL checks should be performed before the dereference. >> >> Spotted by the Coverity checker. >> >> Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> > > This does not apply against 2.6.22-rc1. Please rediff and repost. Ok. Here's a rediff against 2.6.22-rc1. Thanks. -- NULL checks should be performed before the dereference. Spotted by the Coverity checker. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> diff -uprN -X 2.6.22-rc1/Documentation/dontdiff 2.6.22-rc1.orig/drivers/net/wireless/libertas/fw.c 2.6.22-rc1/drivers/net/wireless/libertas/fw.c --- 2.6.22-rc1.orig/drivers/net/wireless/libertas/fw.c 2007-05-19 10:48:02.0 +0800 +++ 2.6.22-rc1/drivers/net/wireless/libertas/fw.c 2007-05-19 11:01:26.0 +0800 @@ -333,18 +333,22 @@ static void command_timer_fn(unsigned lo unsigned long flags; ptempnode = adapter->cur_cmd; + if (ptempnode == NULL) { + lbs_pr_debug(1, "PTempnode Empty\n"); + return; + } + cmd = (struct cmd_ds_command *)ptempnode->bufvirtualaddr; + if (!cmd) { + lbs_pr_debug(1, "cmd is NULL\n"); + return; + } lbs_pr_info("command_timer_fn fired (%x)\n", cmd->command); if (!adapter->fw_ready) return; - if (ptempnode == NULL) { - lbs_pr_debug(1, "PTempnode Empty\n"); - return; - } - spin_lock_irqsave(&adapter->driver_lock, flags); adapter->cur_cmd = NULL; spin_unlock_irqrestore(&adapter->driver_lock, flags); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.6 patch] drivers/net/wireless/libertas/fw.c: fix use-before-check
NULL checks should be performed before the dereference. Spotted by the Coverity checker. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> diff --git a/drivers/net/wireless/libertas/fw.c b/drivers/net/wireless/libertas/fw.c index 441123c..5c63c9b 100644 --- a/drivers/net/wireless/libertas/fw.c +++ b/drivers/net/wireless/libertas/fw.c @@ -333,18 +333,22 @@ static void command_timer_fn(unsigned long data) unsigned long flags; ptempnode = adapter->cur_cmd; + if (ptempnode == NULL) { + lbs_pr_debug(1, "PTempnode Empty\n"); + return; + } + cmd = (struct cmd_ds_command *)ptempnode->bufvirtualaddr; + if (!cmd) { + lbs_pr_debug(1, "cmd is NULL\n"); + return; + } lbs_pr_info("command_timer_fn fired (%x)\n", cmd->command); if (!adapter->fw_ready) return; - if (ptempnode == NULL) { - lbs_pr_debug(1, "PTempnode Empty\n"); - return; - } - spin_lock_irqsave(&adapter->driver_lock, flags); adapter->cur_cmd = NULL; spin_unlock_irqrestore(&adapter->driver_lock, flags); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.6 patch] drivers/net/wireless/libertas/rx.c: fix use-after-free
libertas_upload_rx_packet() calls netif_rx() before returning, and it always return 0. Also within libertas_upload_rx_packet(), it will initialize skb->protocol anyways. Spotted by the Coverity checker. Signed-off-by: Eugene Teo <[EMAIL PROTECTED]> diff --git a/drivers/net/wireless/libertas/rx.c b/drivers/net/wireless/libertas/rx.c index d17924f..1d8d5e4 100644 --- a/drivers/net/wireless/libertas/rx.c +++ b/drivers/net/wireless/libertas/rx.c @@ -269,15 +269,12 @@ int libertas_process_rxed_packet(wlan_private * priv, struct sk_buff *skb) wlan_compute_rssi(priv, p_rx_pd); lbs_pr_debug(1, "RX Data: size of actual packet = %d\n", skb->len); - if (libertas_upload_rx_packet(priv, skb)) { - lbs_pr_debug(1, "RX error: libertas_upload_rx_packet" - " returns failure\n"); - ret = -1; - goto done; - } + priv->stats.rx_bytes += skb->len; priv->stats.rx_packets++; + libertas_upload_rx_packet(priv, skb); + ret = 0; done: LEAVE(); @@ -439,21 +436,14 @@ static int process_rxed_802_11_packet(wlan_private * priv, struct sk_buff *skb) lbs_pr_debug(1, "RX Data: size of actual packet = %d\n", skb->len); - if (libertas_upload_rx_packet(priv, skb)) { - lbs_pr_debug(1, "RX error: libertas_upload_rx_packet " - "returns failure\n"); - ret = -1; - goto done; - } - priv->stats.rx_bytes += skb->len; priv->stats.rx_packets++; + libertas_upload_rx_packet(priv, skb); + ret = 0; done: LEAVE(); - skb->protocol = __constant_htons(0x0019); /* ETH_P_80211_RAW */ - - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [2.6 patch] net/sctp/socket.c: add missing sctp_spin_unlock_irqrestore
Al Viro wrote: On Sun, Nov 26, 2006 at 06:00:53PM +0800, Eugene Teo wrote: This patch adds a missing sctp_spin_unlock_irqrestore when returning from "if(space_left + sctp_spin_unlock_irqrestore(&sctp_local_addr_lock, flags); + return err; } You do realize that it's obviously still badly broken, don't you? copy_to_user() under a spinlock is a recipe for deadlock, especially if you've got interrupts disabled... Realized. Back to drawing board. Eugene -- 1024D/58DF8823 print 47B9 90F6 AE4A 9C51 37E0 D6E1 EA84 C6A2 58DF 8823 main(i) { putchar(182623909 >> (i-1) * 5&31|!!(i<7)<<6) && main(++i); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[2.6 patch] net/sctp/socket.c: add missing sctp_spin_unlock_irqrestore
This patch adds a missing sctp_spin_unlock_irqrestore when returning from "if(space_left net/sctp/socket.c | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 935bc91..a5d4d75 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -3955,7 +3955,7 @@ static int sctp_copy_laddrs_to_user(stru struct sctp_sockaddr_entry *addr; unsigned long flags; union sctp_addr temp; - int cnt = 0; + int cnt = 0, err = 0; int addrlen; sctp_spin_lock_irqsave(&sctp_local_addr_lock, flags); @@ -3968,21 +3968,24 @@ static int sctp_copy_laddrs_to_user(stru sctp_get_pf_specific(sk->sk_family)->addr_v4map(sctp_sk(sk), &temp); addrlen = sctp_get_af_specific(temp.sa.sa_family)->sockaddr_len; - if(space_lefthttp://www.kernel.org/~eugeneteo gpg fingerprint: 47B9 90F6 AE4A 9C51 37E0 D6E1 EA84 C6A2 58DF 8823 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/