[RESEND][PATCH] ARM: mm: Free memblock from free_initrd_mem()
Even after free_initrd_mem(), memblock for initrd remains. Free
memblock for initrd from free_initrd_mem() line generic function.
Reported-by: Jaehoon Chung
Signed-off-by: Seung-Woo Kim
---
arch/arm/mm/init.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 828a256..f7db023 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -530,6 +530,13 @@ void free_initmem(void)
#ifdef CONFIG_BLK_DEV_INITRD
void free_initrd_mem(unsigned long start, unsigned long end)
{
+#ifdef CONFIG_ARCH_KEEP_MEMBLOCK
+ unsigned long aligned_start = ALIGN_DOWN(start, PAGE_SIZE);
+ unsigned long aligned_end = ALIGN(end, PAGE_SIZE);
+
+ memblock_free(__pa(aligned_start), aligned_end - aligned_start);
+#endif
+
if (start == initrd_start)
start = round_down(start, PAGE_SIZE);
if (end == initrd_end)
--
1.7.4.1
[PATCH] ARM: mm: Free memblock from free_initrd_mem()
Even after free_initrd_mem(), memblock for initrd remains. Free
memblock for initrd from free_initrd_mem().
Reported-by: Jaehoon Chung
Signed-off-by: Seung-Woo Kim
---
arch/arm/mm/init.c |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 828a256..f7db023 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -530,6 +530,13 @@ void free_initmem(void)
#ifdef CONFIG_BLK_DEV_INITRD
void free_initrd_mem(unsigned long start, unsigned long end)
{
+#ifdef CONFIG_ARCH_KEEP_MEMBLOCK
+ unsigned long aligned_start = ALIGN_DOWN(start, PAGE_SIZE);
+ unsigned long aligned_end = ALIGN(end, PAGE_SIZE);
+
+ memblock_free(__pa(aligned_start), aligned_end - aligned_start);
+#endif
+
if (start == initrd_start)
start = round_down(start, PAGE_SIZE);
if (end == initrd_end)
--
1.7.4.1
Re: Linux 3.18.111
On 2018년 08월 10일 19:11, Greg Kroah-Hartman wrote:
> On Fri, Aug 10, 2018 at 03:43:02PM +0900, Seung-Woo Kim wrote:
>> On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
>>> On 2018년 07월 05일 09:52, Al Viro wrote:
>>>> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>>>>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim
>>>>> wrote:
>>>>>>
>>>>>> I think the commit itself is required. Simple, but not reliable,
>>>>>> workaround fix is like below:
>>>>>>
>>>>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>>>>> index a34d401..7c751f2 100644
>>>>>> --- a/fs/dcache.c
>>>>>> +++ b/fs/dcache.c
>>>>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>>>>> struct inode *inode)
>>>>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>>>>> BUG_ON(!inode);
>>>>>> lockdep_annotate_inode_mutex_key(inode);
>>>>>> + /* WORKAROUND for calling security_d_instantiate() */
>>>>>> + entry->d_inode = inode;
>>>>>> security_d_instantiate(entry, inode);
>>>>>> spin_lock(>i_lock);
>>>>>> __d_instantiate(entry, inode);
>>>>>
>>>>> Ugh. That looks horrible even if it might avoid the oops.
>>>>>
>>>>> I think a much better solution is to back-port commit b296821a7c42
>>>>> ("xattr_handler: pass dentry and inode as separate arguments of
>>>>> ->get()") to older kernels. Then the inode is passed down all the way,
>>>>> and you don't have people try to get it from the (not yet initialized)
>>>>> dentry.
>>>>>
>>>>> But there might be other parts missing too, and I didn't look at how
>>>>> easy/painful that backport would be.
>>>>>
>>>>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>>>>> d_instantiate/unlock_new_inode combinations safely") being marked for
>>>>> stable, and various cases of security_d_instantiate() calling down to
>>>>> getxattr. Which used to not get the inode at all, so those older
>>>>> kernels use d_inode(dentry), which doesn't work in this path since
>>>>> dentry->d_inode hasn't been instantiated yet..
>>>>
>>>> You also want b96809173e94 and ce23e6401334 there...
>>>
>>> For above two commits, also b296821a7c42 is required. And after
>>> backport, smack still crashed because setxattr. To fix it, 5930122683df
>>> and 3767e255b390 are also required.
>>>
>>> By the way, does no one have met this kind getxattr crash issue with
>>> selinux from 3.18.y?
>>>
>>
>> I have checked with selinux, and it is confirmed that there is no crash
>> because selinux_d_instantiate() has null check for inode. So, it is only
>> security smack issue.
>
> So are the 5 patches you sent ok to apply to the 3.18-stable tree? Or
> do we need to do something else?
>
Those 5 patches are fine in my smack environment. I have not tested all
the file systems in run-time except ext2/4 and I only tested build for
those file systems.
Best Regards,
- Seung-Woo Kim
> thanks,
>
> greg k-h
>
>
Re: Linux 3.18.111
On 2018년 08월 10일 19:11, Greg Kroah-Hartman wrote:
> On Fri, Aug 10, 2018 at 03:43:02PM +0900, Seung-Woo Kim wrote:
>> On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
>>> On 2018년 07월 05일 09:52, Al Viro wrote:
>>>> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>>>>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim
>>>>> wrote:
>>>>>>
>>>>>> I think the commit itself is required. Simple, but not reliable,
>>>>>> workaround fix is like below:
>>>>>>
>>>>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>>>>> index a34d401..7c751f2 100644
>>>>>> --- a/fs/dcache.c
>>>>>> +++ b/fs/dcache.c
>>>>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>>>>> struct inode *inode)
>>>>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>>>>> BUG_ON(!inode);
>>>>>> lockdep_annotate_inode_mutex_key(inode);
>>>>>> + /* WORKAROUND for calling security_d_instantiate() */
>>>>>> + entry->d_inode = inode;
>>>>>> security_d_instantiate(entry, inode);
>>>>>> spin_lock(>i_lock);
>>>>>> __d_instantiate(entry, inode);
>>>>>
>>>>> Ugh. That looks horrible even if it might avoid the oops.
>>>>>
>>>>> I think a much better solution is to back-port commit b296821a7c42
>>>>> ("xattr_handler: pass dentry and inode as separate arguments of
>>>>> ->get()") to older kernels. Then the inode is passed down all the way,
>>>>> and you don't have people try to get it from the (not yet initialized)
>>>>> dentry.
>>>>>
>>>>> But there might be other parts missing too, and I didn't look at how
>>>>> easy/painful that backport would be.
>>>>>
>>>>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>>>>> d_instantiate/unlock_new_inode combinations safely") being marked for
>>>>> stable, and various cases of security_d_instantiate() calling down to
>>>>> getxattr. Which used to not get the inode at all, so those older
>>>>> kernels use d_inode(dentry), which doesn't work in this path since
>>>>> dentry->d_inode hasn't been instantiated yet..
>>>>
>>>> You also want b96809173e94 and ce23e6401334 there...
>>>
>>> For above two commits, also b296821a7c42 is required. And after
>>> backport, smack still crashed because setxattr. To fix it, 5930122683df
>>> and 3767e255b390 are also required.
>>>
>>> By the way, does no one have met this kind getxattr crash issue with
>>> selinux from 3.18.y?
>>>
>>
>> I have checked with selinux, and it is confirmed that there is no crash
>> because selinux_d_instantiate() has null check for inode. So, it is only
>> security smack issue.
>
> So are the 5 patches you sent ok to apply to the 3.18-stable tree? Or
> do we need to do something else?
>
Those 5 patches are fine in my smack environment. I have not tested all
the file systems in run-time except ext2/4 and I only tested build for
those file systems.
Best Regards,
- Seung-Woo Kim
> thanks,
>
> greg k-h
>
>
Re: Linux 3.18.111
On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
> On 2018년 07월 05일 09:52, Al Viro wrote:
>> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim wrote:
>>>>
>>>> I think the commit itself is required. Simple, but not reliable,
>>>> workaround fix is like below:
>>>>
>>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>>> index a34d401..7c751f2 100644
>>>> --- a/fs/dcache.c
>>>> +++ b/fs/dcache.c
>>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>>> struct inode *inode)
>>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>>> BUG_ON(!inode);
>>>> lockdep_annotate_inode_mutex_key(inode);
>>>> + /* WORKAROUND for calling security_d_instantiate() */
>>>> + entry->d_inode = inode;
>>>> security_d_instantiate(entry, inode);
>>>> spin_lock(>i_lock);
>>>> __d_instantiate(entry, inode);
>>>
>>> Ugh. That looks horrible even if it might avoid the oops.
>>>
>>> I think a much better solution is to back-port commit b296821a7c42
>>> ("xattr_handler: pass dentry and inode as separate arguments of
>>> ->get()") to older kernels. Then the inode is passed down all the way,
>>> and you don't have people try to get it from the (not yet initialized)
>>> dentry.
>>>
>>> But there might be other parts missing too, and I didn't look at how
>>> easy/painful that backport would be.
>>>
>>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>>> d_instantiate/unlock_new_inode combinations safely") being marked for
>>> stable, and various cases of security_d_instantiate() calling down to
>>> getxattr. Which used to not get the inode at all, so those older
>>> kernels use d_inode(dentry), which doesn't work in this path since
>>> dentry->d_inode hasn't been instantiated yet..
>>
>> You also want b96809173e94 and ce23e6401334 there...
>
> For above two commits, also b296821a7c42 is required. And after
> backport, smack still crashed because setxattr. To fix it, 5930122683df
> and 3767e255b390 are also required.
>
> By the way, does no one have met this kind getxattr crash issue with
> selinux from 3.18.y?
>
I have checked with selinux, and it is confirmed that there is no crash
because selinux_d_instantiate() has null check for inode. So, it is only
security smack issue.
--
Seung-Woo Kim
Samsung Research
--
Re: Linux 3.18.111
On 2018년 08월 08일 19:06, Seung-Woo Kim wrote:
> On 2018년 07월 05일 09:52, Al Viro wrote:
>> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim wrote:
>>>>
>>>> I think the commit itself is required. Simple, but not reliable,
>>>> workaround fix is like below:
>>>>
>>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>>> index a34d401..7c751f2 100644
>>>> --- a/fs/dcache.c
>>>> +++ b/fs/dcache.c
>>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>>> struct inode *inode)
>>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>>> BUG_ON(!inode);
>>>> lockdep_annotate_inode_mutex_key(inode);
>>>> + /* WORKAROUND for calling security_d_instantiate() */
>>>> + entry->d_inode = inode;
>>>> security_d_instantiate(entry, inode);
>>>> spin_lock(>i_lock);
>>>> __d_instantiate(entry, inode);
>>>
>>> Ugh. That looks horrible even if it might avoid the oops.
>>>
>>> I think a much better solution is to back-port commit b296821a7c42
>>> ("xattr_handler: pass dentry and inode as separate arguments of
>>> ->get()") to older kernels. Then the inode is passed down all the way,
>>> and you don't have people try to get it from the (not yet initialized)
>>> dentry.
>>>
>>> But there might be other parts missing too, and I didn't look at how
>>> easy/painful that backport would be.
>>>
>>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>>> d_instantiate/unlock_new_inode combinations safely") being marked for
>>> stable, and various cases of security_d_instantiate() calling down to
>>> getxattr. Which used to not get the inode at all, so those older
>>> kernels use d_inode(dentry), which doesn't work in this path since
>>> dentry->d_inode hasn't been instantiated yet..
>>
>> You also want b96809173e94 and ce23e6401334 there...
>
> For above two commits, also b296821a7c42 is required. And after
> backport, smack still crashed because setxattr. To fix it, 5930122683df
> and 3767e255b390 are also required.
>
> By the way, does no one have met this kind getxattr crash issue with
> selinux from 3.18.y?
>
I have checked with selinux, and it is confirmed that there is no crash
because selinux_d_instantiate() has null check for inode. So, it is only
security smack issue.
--
Seung-Woo Kim
Samsung Research
--
[PATCH 3.18.y 5/5] switch ->setxattr() to passing dentry and inode separately
From: Al Viro
smack ->d_instantiate() uses ->setxattr(), so to be able to call it before
we'd hashed the new dentry and attached it to inode, we need ->setxattr()
instances getting the inode as an explicit argument rather than obtaining
it from dentry.
Similar change for ->getxattr() had been done in commit ce23e64. Unlike
->getxattr() (which is used by both selinux and smack instances of
->d_instantiate()) ->setxattr() is used only by smack one and unfortunately
it got missed back then.
Reported-by: Seung-Woo Kim
Tested-by: Casey Schaufler
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
It is not clear, a26feccaba29 ("ceph: Get rid of d_find_alias in ceph_set_acl")
is required backport for ceph or not.
---
Documentation/filesystems/porting |7 +++
.../staging/lustre/lustre/llite/llite_internal.h |4 ++--
drivers/staging/lustre/lustre/llite/xattr.c|6 ++
fs/bad_inode.c |4 ++--
fs/btrfs/xattr.c | 12 +++-
fs/btrfs/xattr.h |5 +++--
fs/ceph/super.h|4 ++--
fs/ceph/xattr.c|7 ---
fs/cifs/cifsfs.h |4 ++--
fs/cifs/xattr.c| 13 +++--
fs/ecryptfs/crypto.c |9 +
fs/ecryptfs/ecryptfs_kernel.h |4 ++--
fs/ecryptfs/inode.c|7 ---
fs/ecryptfs/mmap.c |3 ++-
fs/fuse/dir.c |6 +++---
fs/gfs2/inode.c|9 +
fs/hfs/attr.c |6 +++---
fs/hfs/hfs_fs.h|2 +-
fs/jfs/jfs_xattr.h |4 ++--
fs/jfs/xattr.c | 10 +-
fs/kernfs/inode.c | 11 ++-
fs/kernfs/kernfs-internal.h|3 ++-
fs/overlayfs/inode.c |5 +++--
fs/overlayfs/overlayfs.h |5 +++--
fs/reiserfs/xattr.c|8
fs/reiserfs/xattr.h|5 +++--
fs/ubifs/ubifs.h |2 +-
fs/ubifs/xattr.c |4 ++--
fs/xattr.c |9 +
include/linux/fs.h |3 ++-
include/linux/xattr.h |3 ++-
mm/shmem.c | 10 ++
security/smack/smack_lsm.c |2 +-
33 files changed, 110 insertions(+), 86 deletions(-)
diff --git a/Documentation/filesystems/porting
b/Documentation/filesystems/porting
index daf9acd..51bdd4a 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -469,3 +469,10 @@ in your dentry operations instead.
dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
called before we attach dentry to inode.
+--
+[mandatory]
+ ->setxattr() and xattr_handler.set() get dentry and inode passed
separately.
+ dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
+ in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
+ called before we attach dentry to inode and !@#!@##!@$!$#!@#$!@$!@$
smack
+ ->d_instantiate() uses not just ->getxattr() but ->setxattr() as well.
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 442fe5b..d7ac9bc 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -1068,8 +1068,8 @@ static inline __u64 ll_file_maxbytes(struct inode *inode)
}
/* llite/xattr.c */
-int ll_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags);
+int ll_setxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value, size_t size, int flags);
ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
const char *name, void *buffer, size_t size);
ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size);
diff --git a/drivers/staging/lustre/lustre/llite/xattr.c
b/drivers/staging/lustre/lustre/llite/xattr.c
index fc4b95d3..3edbbd2 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lus
[PATCH 3.18.y 5/5] switch ->setxattr() to passing dentry and inode separately
From: Al Viro
smack ->d_instantiate() uses ->setxattr(), so to be able to call it before
we'd hashed the new dentry and attached it to inode, we need ->setxattr()
instances getting the inode as an explicit argument rather than obtaining
it from dentry.
Similar change for ->getxattr() had been done in commit ce23e64. Unlike
->getxattr() (which is used by both selinux and smack instances of
->d_instantiate()) ->setxattr() is used only by smack one and unfortunately
it got missed back then.
Reported-by: Seung-Woo Kim
Tested-by: Casey Schaufler
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
It is not clear, a26feccaba29 ("ceph: Get rid of d_find_alias in ceph_set_acl")
is required backport for ceph or not.
---
Documentation/filesystems/porting |7 +++
.../staging/lustre/lustre/llite/llite_internal.h |4 ++--
drivers/staging/lustre/lustre/llite/xattr.c|6 ++
fs/bad_inode.c |4 ++--
fs/btrfs/xattr.c | 12 +++-
fs/btrfs/xattr.h |5 +++--
fs/ceph/super.h|4 ++--
fs/ceph/xattr.c|7 ---
fs/cifs/cifsfs.h |4 ++--
fs/cifs/xattr.c| 13 +++--
fs/ecryptfs/crypto.c |9 +
fs/ecryptfs/ecryptfs_kernel.h |4 ++--
fs/ecryptfs/inode.c|7 ---
fs/ecryptfs/mmap.c |3 ++-
fs/fuse/dir.c |6 +++---
fs/gfs2/inode.c|9 +
fs/hfs/attr.c |6 +++---
fs/hfs/hfs_fs.h|2 +-
fs/jfs/jfs_xattr.h |4 ++--
fs/jfs/xattr.c | 10 +-
fs/kernfs/inode.c | 11 ++-
fs/kernfs/kernfs-internal.h|3 ++-
fs/overlayfs/inode.c |5 +++--
fs/overlayfs/overlayfs.h |5 +++--
fs/reiserfs/xattr.c|8
fs/reiserfs/xattr.h|5 +++--
fs/ubifs/ubifs.h |2 +-
fs/ubifs/xattr.c |4 ++--
fs/xattr.c |9 +
include/linux/fs.h |3 ++-
include/linux/xattr.h |3 ++-
mm/shmem.c | 10 ++
security/smack/smack_lsm.c |2 +-
33 files changed, 110 insertions(+), 86 deletions(-)
diff --git a/Documentation/filesystems/porting
b/Documentation/filesystems/porting
index daf9acd..51bdd4a 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -469,3 +469,10 @@ in your dentry operations instead.
dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
called before we attach dentry to inode.
+--
+[mandatory]
+ ->setxattr() and xattr_handler.set() get dentry and inode passed
separately.
+ dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
+ in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
+ called before we attach dentry to inode and !@#!@##!@$!$#!@#$!@$!@$
smack
+ ->d_instantiate() uses not just ->getxattr() but ->setxattr() as well.
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 442fe5b..d7ac9bc 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -1068,8 +1068,8 @@ static inline __u64 ll_file_maxbytes(struct inode *inode)
}
/* llite/xattr.c */
-int ll_setxattr(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags);
+int ll_setxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value, size_t size, int flags);
ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
const char *name, void *buffer, size_t size);
ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size);
diff --git a/drivers/staging/lustre/lustre/llite/xattr.c
b/drivers/staging/lustre/lustre/llite/xattr.c
index fc4b95d3..3edbbd2 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lus
[PATCH 3.18.y 2/5] ->getxattr(): pass dentry and inode as separate arguments
From: Al Viro
commit ce23e640133484eebc20ca7b7668388213e11327 upstream.
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
Documentation/filesystems/porting |6 +
.../staging/lustre/lustre/llite/llite_internal.h |4 +-
drivers/staging/lustre/lustre/llite/xattr.c|6 +---
fs/bad_inode.c |4 +-
fs/btrfs/xattr.c |8 +++---
fs/btrfs/xattr.h |4 +-
fs/ceph/super.h|3 +-
fs/ceph/xattr.c|8 +++---
fs/cifs/cifsfs.h |2 +-
fs/cifs/xattr.c| 10
fs/ecryptfs/crypto.c |5 +++-
fs/ecryptfs/ecryptfs_kernel.h |4 +-
fs/ecryptfs/inode.c| 23 ++-
fs/ecryptfs/mmap.c |3 +-
fs/fuse/dir.c |5 +--
fs/gfs2/inode.c|9 +++
fs/hfs/attr.c |5 +--
fs/hfs/hfs_fs.h|4 +-
fs/jfs/jfs_xattr.h |2 +-
fs/jfs/xattr.c |8 +++---
fs/kernfs/inode.c |6 ++--
fs/kernfs/kernfs-internal.h|4 +-
fs/overlayfs/inode.c |4 +-
fs/overlayfs/overlayfs.h |4 +-
fs/overlayfs/super.c |2 +-
fs/reiserfs/xattr.c|8 +++---
fs/reiserfs/xattr.h|4 +-
fs/ubifs/ubifs.h |4 +-
fs/ubifs/xattr.c |6 ++--
fs/xattr.c | 13 ++-
include/linux/fs.h |3 +-
include/linux/xattr.h |2 +-
mm/shmem.c |8 +++---
net/socket.c |2 +-
security/commoncap.c |6 ++--
security/integrity/evm/evm_main.c |2 +-
security/selinux/hooks.c |9 ---
security/smack/smack_lsm.c |4 +-
38 files changed, 112 insertions(+), 102 deletions(-)
diff --git a/Documentation/filesystems/porting
b/Documentation/filesystems/porting
index 0f3a139..daf9acd 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -463,3 +463,9 @@ in your dentry operations instead.
of the in-tree instances did). inode_hash_lock is still held,
of course, so they are still serialized wrt removal from inode hash,
as well as wrt set() callback of iget5_locked().
+--
+[mandatory]
+ ->getxattr() and xattr_handler.get() get dentry and inode passed
separately.
+ dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
+ in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
+ called before we attach dentry to inode.
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 36aa0fd..442fe5b 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -1070,8 +1070,8 @@ static inline __u64 ll_file_maxbytes(struct inode *inode)
/* llite/xattr.c */
int ll_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
-ssize_t ll_getxattr(struct dentry *dentry, const char *name,
- void *buffer, size_t size);
+ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size);
ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size);
int ll_removexattr(struct dentry *dentry, const char *name);
diff --git a/drivers/staging/lustre/lustre/llite/xattr.c
b/drivers/staging/lustre/lustre/llite/xattr.c
index 252a619..fc4b95d3 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -455,11 +455,9 @@ out:
return rc;
}
-ssize_t ll_getxattr(struct dentry *dentry, const char *name,
- void *buffer, size_t size)
+ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
{
- struct inode *inode = dentry->d_inode;
-
LASSERT(inode);
LASSERT(name);
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index afd2b44..336c2
[PATCH 3.18.y 2/5] ->getxattr(): pass dentry and inode as separate arguments
From: Al Viro
commit ce23e640133484eebc20ca7b7668388213e11327 upstream.
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
Documentation/filesystems/porting |6 +
.../staging/lustre/lustre/llite/llite_internal.h |4 +-
drivers/staging/lustre/lustre/llite/xattr.c|6 +---
fs/bad_inode.c |4 +-
fs/btrfs/xattr.c |8 +++---
fs/btrfs/xattr.h |4 +-
fs/ceph/super.h|3 +-
fs/ceph/xattr.c|8 +++---
fs/cifs/cifsfs.h |2 +-
fs/cifs/xattr.c| 10
fs/ecryptfs/crypto.c |5 +++-
fs/ecryptfs/ecryptfs_kernel.h |4 +-
fs/ecryptfs/inode.c| 23 ++-
fs/ecryptfs/mmap.c |3 +-
fs/fuse/dir.c |5 +--
fs/gfs2/inode.c|9 +++
fs/hfs/attr.c |5 +--
fs/hfs/hfs_fs.h|4 +-
fs/jfs/jfs_xattr.h |2 +-
fs/jfs/xattr.c |8 +++---
fs/kernfs/inode.c |6 ++--
fs/kernfs/kernfs-internal.h|4 +-
fs/overlayfs/inode.c |4 +-
fs/overlayfs/overlayfs.h |4 +-
fs/overlayfs/super.c |2 +-
fs/reiserfs/xattr.c|8 +++---
fs/reiserfs/xattr.h|4 +-
fs/ubifs/ubifs.h |4 +-
fs/ubifs/xattr.c |6 ++--
fs/xattr.c | 13 ++-
include/linux/fs.h |3 +-
include/linux/xattr.h |2 +-
mm/shmem.c |8 +++---
net/socket.c |2 +-
security/commoncap.c |6 ++--
security/integrity/evm/evm_main.c |2 +-
security/selinux/hooks.c |9 ---
security/smack/smack_lsm.c |4 +-
38 files changed, 112 insertions(+), 102 deletions(-)
diff --git a/Documentation/filesystems/porting
b/Documentation/filesystems/porting
index 0f3a139..daf9acd 100644
--- a/Documentation/filesystems/porting
+++ b/Documentation/filesystems/porting
@@ -463,3 +463,9 @@ in your dentry operations instead.
of the in-tree instances did). inode_hash_lock is still held,
of course, so they are still serialized wrt removal from inode hash,
as well as wrt set() callback of iget5_locked().
+--
+[mandatory]
+ ->getxattr() and xattr_handler.get() get dentry and inode passed
separately.
+ dentry might be yet to be attached to inode, so do _not_ use its
->d_inode
+ in the instances. Rationale: !@#!@# security_d_instantiate() needs to
be
+ called before we attach dentry to inode.
diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h
b/drivers/staging/lustre/lustre/llite/llite_internal.h
index 36aa0fd..442fe5b 100644
--- a/drivers/staging/lustre/lustre/llite/llite_internal.h
+++ b/drivers/staging/lustre/lustre/llite/llite_internal.h
@@ -1070,8 +1070,8 @@ static inline __u64 ll_file_maxbytes(struct inode *inode)
/* llite/xattr.c */
int ll_setxattr(struct dentry *dentry, const char *name,
const void *value, size_t size, int flags);
-ssize_t ll_getxattr(struct dentry *dentry, const char *name,
- void *buffer, size_t size);
+ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size);
ssize_t ll_listxattr(struct dentry *dentry, char *buffer, size_t size);
int ll_removexattr(struct dentry *dentry, const char *name);
diff --git a/drivers/staging/lustre/lustre/llite/xattr.c
b/drivers/staging/lustre/lustre/llite/xattr.c
index 252a619..fc4b95d3 100644
--- a/drivers/staging/lustre/lustre/llite/xattr.c
+++ b/drivers/staging/lustre/lustre/llite/xattr.c
@@ -455,11 +455,9 @@ out:
return rc;
}
-ssize_t ll_getxattr(struct dentry *dentry, const char *name,
- void *buffer, size_t size)
+ssize_t ll_getxattr(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size)
{
- struct inode *inode = dentry->d_inode;
-
LASSERT(inode);
LASSERT(name);
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index afd2b44..336c2
[PATCH 3.18.y 1/5] xattr_handler: pass dentry and inode as separate arguments of ->get()
From: Al Viro
commit b296821a7c42fa58baa17513b2b7b30ae66f3336 upstream.
... and do not assume they are already attached to each other
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18 including ext3]
Signed-off-by: Seung-Woo Kim
---
fs/9p/acl.c |7 ---
fs/9p/xattr_security.c |5 +++--
fs/9p/xattr_trusted.c|5 +++--
fs/9p/xattr_user.c |5 +++--
fs/ext2/xattr_security.c |6 +++---
fs/ext2/xattr_trusted.c |6 +++---
fs/ext2/xattr_user.c |8
fs/ext3/xattr_security.c |6 +++---
fs/ext3/xattr_trusted.c |6 +++---
fs/ext3/xattr_user.c |8
fs/ext4/xattr_security.c |6 +++---
fs/ext4/xattr_trusted.c |6 +++---
fs/ext4/xattr_user.c |8
fs/f2fs/xattr.c | 15 +++
fs/gfs2/xattr.c |6 +++---
fs/hfsplus/xattr.c |7 ---
fs/hfsplus/xattr.h | 10 +-
fs/hfsplus/xattr_security.c |7 ---
fs/hfsplus/xattr_trusted.c |7 ---
fs/hfsplus/xattr_user.c |7 ---
fs/jffs2/security.c |7 ---
fs/jffs2/xattr_trusted.c |7 ---
fs/jffs2/xattr_user.c|7 ---
fs/nfs/nfs4proc.c| 14 --
fs/ocfs2/xattr.c | 23 +--
fs/posix_acl.c | 10 +-
fs/reiserfs/xattr.c |3 ++-
fs/reiserfs/xattr_security.c |8
fs/reiserfs/xattr_trusted.c |8
fs/reiserfs/xattr_user.c |8
fs/squashfs/xattr.c | 22 +-
fs/xattr.c |3 ++-
fs/xfs/xfs_xattr.c |4 ++--
include/linux/xattr.h|4 ++--
34 files changed, 145 insertions(+), 124 deletions(-)
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index cc67477..938e1a4 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -230,8 +230,9 @@ static int v9fs_remote_get_acl(struct dentry *dentry, const
char *name,
return v9fs_xattr_get(dentry, full_name, buffer, size);
}
-static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_get_acl(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
struct v9fs_session_info *v9ses;
struct posix_acl *acl;
@@ -247,7 +248,7 @@ static int v9fs_xattr_get_acl(struct dentry *dentry, const
char *name,
if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
return v9fs_remote_get_acl(dentry, name, buffer, size, type);
- acl = v9fs_get_cached_acl(dentry->d_inode, type);
+ acl = v9fs_get_cached_acl(inode, type);
if (IS_ERR(acl))
return PTR_ERR(acl);
if (acl == NULL)
diff --git a/fs/9p/xattr_security.c b/fs/9p/xattr_security.c
index cb247a1..df692f9 100644
--- a/fs/9p/xattr_security.c
+++ b/fs/9p/xattr_security.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_security_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_security_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_trusted.c b/fs/9p/xattr_trusted.c
index e30d33b..acfadcf 100644
--- a/fs/9p/xattr_trusted.c
+++ b/fs/9p/xattr_trusted.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_trusted_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_trusted_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_user.c b/fs/9p/xattr_user.c
index d0b701b..9b37675 100644
--- a/fs/9p/xattr_user.c
+++ b/fs/9p/xattr_user.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_user_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_user_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index c0ebc4d..1d53da4 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -23,12 +23,12 @@ ext2_xattr_security_list(struct dentry *dentry, char *list,
size_t list_size,
[PATCH 3.18.y 4/5] switch xattr_handler->set() to passing dentry and inode separately
From: Al Viro
preparation for similar switch in ->setxattr() (see the next commit for
rationale).
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18 including ext3]
Signed-off-by: Seung-Woo Kim
---
fs/9p/acl.c |7 +++
fs/9p/xattr_security.c |5 +++--
fs/9p/xattr_trusted.c|5 +++--
fs/9p/xattr_user.c |5 +++--
fs/ext2/xattr_security.c |7 ---
fs/ext2/xattr_trusted.c |7 ---
fs/ext2/xattr_user.c |9 +
fs/ext3/xattr_security.c |7 ---
fs/ext3/xattr_trusted.c |7 ---
fs/ext3/xattr_user.c |9 +
fs/ext4/xattr_security.c |7 ---
fs/ext4/xattr_trusted.c |7 ---
fs/ext4/xattr_user.c |9 +
fs/f2fs/xattr.c | 17 -
fs/gfs2/xattr.c |7 ---
fs/hfsplus/xattr.c |7 ---
fs/hfsplus/xattr.h |4 ++--
fs/hfsplus/xattr_security.c |7 ---
fs/hfsplus/xattr_trusted.c |7 ---
fs/hfsplus/xattr_user.c |7 ---
fs/jffs2/security.c |7 ---
fs/jffs2/xattr_trusted.c |7 ---
fs/jffs2/xattr_user.c|7 ---
fs/nfs/nfs4proc.c| 19 +--
fs/ocfs2/xattr.c | 23 +--
fs/posix_acl.c |6 +++---
fs/reiserfs/xattr.c |6 --
fs/reiserfs/xattr_security.c |8
fs/reiserfs/xattr_trusted.c |8
fs/reiserfs/xattr_user.c |8
fs/xattr.c |5 +++--
fs/xfs/xfs_xattr.c |7 ---
include/linux/xattr.h|5 +++--
33 files changed, 144 insertions(+), 119 deletions(-)
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 938e1a4..0928d74 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -279,14 +279,13 @@ static int v9fs_remote_set_acl(struct dentry *dentry,
const char *name,
}
-static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
- const void *value, size_t size,
- int flags, int type)
+static int v9fs_xattr_set_acl(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
struct posix_acl *acl;
struct v9fs_session_info *v9ses;
- struct inode *inode = dentry->d_inode;
if (strcmp(name, "") != 0)
return -EINVAL;
diff --git a/fs/9p/xattr_security.c b/fs/9p/xattr_security.c
index df692f9..242c1f8 100644
--- a/fs/9p/xattr_security.c
+++ b/fs/9p/xattr_security.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_security_get(struct dentry *dentry,
struct inode *inode,
return retval;
}
-static int v9fs_xattr_security_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_security_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_trusted.c b/fs/9p/xattr_trusted.c
index acfadcf..a1fe7d9 100644
--- a/fs/9p/xattr_trusted.c
+++ b/fs/9p/xattr_trusted.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_trusted_get(struct dentry *dentry,
struct inode *inode,
return retval;
}
-static int v9fs_xattr_trusted_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_trusted_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_user.c b/fs/9p/xattr_user.c
index 9b37675..7a34aca 100644
--- a/fs/9p/xattr_user.c
+++ b/fs/9p/xattr_user.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_user_get(struct dentry *dentry, struct
inode *inode,
return retval;
}
-static int v9fs_xattr_user_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_user_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index 1d53da4..3ece525 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -33,12 +33,13 @@ ext2_xattr_security_get(struct dentry *unused, struct inode
*inode,
}
static int
-ext2_xattr_security_set(struct dentry *dentry, const char *name,
-
[PATCH 3.18.y 3/5] security_d_instantiate(): move to the point prior to attaching dentry to inode
From: Al Viro
commit b96809173e94ea2fa8c19c2e40e8545a1821bf57 upstream.
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
fs/dcache.c | 18 +++---
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index a34d401..6a4290a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1729,12 +1729,12 @@ static void __d_instantiate(struct dentry *dentry,
struct inode *inode)
void d_instantiate(struct dentry *entry, struct inode * inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
if (inode)
spin_lock(>i_lock);
__d_instantiate(entry, inode);
if (inode)
spin_unlock(>i_lock);
- security_d_instantiate(entry, inode);
}
EXPORT_SYMBOL(d_instantiate);
@@ -1795,16 +1795,15 @@ struct dentry *d_instantiate_unique(struct dentry
*entry, struct inode *inode)
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
if (inode)
spin_lock(>i_lock);
result = __d_instantiate_unique(entry, inode);
if (inode)
spin_unlock(>i_lock);
- if (!result) {
- security_d_instantiate(entry, inode);
+ if (!result)
return NULL;
- }
BUG_ON(!d_unhashed(result));
iput(inode);
@@ -1826,6 +1825,7 @@ int d_instantiate_no_diralias(struct dentry *entry,
struct inode *inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
spin_lock(>i_lock);
if (S_ISDIR(inode->i_mode) && !hlist_empty(>i_dentry)) {
spin_unlock(>i_lock);
@@ -1834,7 +1834,6 @@ int d_instantiate_no_diralias(struct dentry *entry,
struct inode *inode)
}
__d_instantiate(entry, inode);
spin_unlock(>i_lock);
- security_d_instantiate(entry, inode);
return 0;
}
@@ -1930,6 +1929,7 @@ static struct dentry *__d_obtain_alias(struct inode
*inode, int disconnected)
goto out_iput;
}
+ security_d_instantiate(tmp, inode);
spin_lock(>i_lock);
res = __d_find_any_alias(inode);
if (res) {
@@ -1953,13 +1953,10 @@ static struct dentry *__d_obtain_alias(struct inode
*inode, int disconnected)
hlist_bl_unlock(>d_sb->s_anon);
spin_unlock(>d_lock);
spin_unlock(>i_lock);
- security_d_instantiate(tmp, inode);
return tmp;
out_iput:
- if (res && !IS_ERR(res))
- security_d_instantiate(res, inode);
iput(inode);
return res;
}
@@ -2817,6 +2814,7 @@ struct dentry *d_splice_alias(struct inode *inode, struct
dentry *dentry)
return ERR_CAST(inode);
if (inode && S_ISDIR(inode->i_mode)) {
+ security_d_instantiate(dentry, inode);
spin_lock(>i_lock);
new = __d_find_any_alias(inode);
if (new) {
@@ -2836,13 +2834,11 @@ struct dentry *d_splice_alias(struct inode *inode,
struct dentry *dentry)
__d_move(new, dentry, false);
write_sequnlock(_lock);
spin_unlock(>i_lock);
- security_d_instantiate(new, inode);
iput(inode);
} else {
/* already taking inode->i_lock, so d_add() by hand */
__d_instantiate(dentry, inode);
spin_unlock(>i_lock);
- security_d_instantiate(dentry, inode);
d_rehash(dentry);
}
} else {
@@ -2876,6 +2872,7 @@ struct dentry *d_materialise_unique(struct dentry
*dentry, struct inode *inode)
goto out_nolock;
}
+ security_d_instantiate(dentry, inode);
spin_lock(>i_lock);
if (S_ISDIR(inode->i_mode)) {
@@ -2927,7 +2924,6 @@ found:
spin_unlock(>i_lock);
out_nolock:
if (actual == dentry) {
- security_d_instantiate(dentry, inode);
return NULL;
}
--
1.7.4.1
[PATCH 3.18.y 1/5] xattr_handler: pass dentry and inode as separate arguments of ->get()
From: Al Viro
commit b296821a7c42fa58baa17513b2b7b30ae66f3336 upstream.
... and do not assume they are already attached to each other
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18 including ext3]
Signed-off-by: Seung-Woo Kim
---
fs/9p/acl.c |7 ---
fs/9p/xattr_security.c |5 +++--
fs/9p/xattr_trusted.c|5 +++--
fs/9p/xattr_user.c |5 +++--
fs/ext2/xattr_security.c |6 +++---
fs/ext2/xattr_trusted.c |6 +++---
fs/ext2/xattr_user.c |8
fs/ext3/xattr_security.c |6 +++---
fs/ext3/xattr_trusted.c |6 +++---
fs/ext3/xattr_user.c |8
fs/ext4/xattr_security.c |6 +++---
fs/ext4/xattr_trusted.c |6 +++---
fs/ext4/xattr_user.c |8
fs/f2fs/xattr.c | 15 +++
fs/gfs2/xattr.c |6 +++---
fs/hfsplus/xattr.c |7 ---
fs/hfsplus/xattr.h | 10 +-
fs/hfsplus/xattr_security.c |7 ---
fs/hfsplus/xattr_trusted.c |7 ---
fs/hfsplus/xattr_user.c |7 ---
fs/jffs2/security.c |7 ---
fs/jffs2/xattr_trusted.c |7 ---
fs/jffs2/xattr_user.c|7 ---
fs/nfs/nfs4proc.c| 14 --
fs/ocfs2/xattr.c | 23 +--
fs/posix_acl.c | 10 +-
fs/reiserfs/xattr.c |3 ++-
fs/reiserfs/xattr_security.c |8
fs/reiserfs/xattr_trusted.c |8
fs/reiserfs/xattr_user.c |8
fs/squashfs/xattr.c | 22 +-
fs/xattr.c |3 ++-
fs/xfs/xfs_xattr.c |4 ++--
include/linux/xattr.h|4 ++--
34 files changed, 145 insertions(+), 124 deletions(-)
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index cc67477..938e1a4 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -230,8 +230,9 @@ static int v9fs_remote_get_acl(struct dentry *dentry, const
char *name,
return v9fs_xattr_get(dentry, full_name, buffer, size);
}
-static int v9fs_xattr_get_acl(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_get_acl(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
struct v9fs_session_info *v9ses;
struct posix_acl *acl;
@@ -247,7 +248,7 @@ static int v9fs_xattr_get_acl(struct dentry *dentry, const
char *name,
if ((v9ses->flags & V9FS_ACCESS_MASK) != V9FS_ACCESS_CLIENT)
return v9fs_remote_get_acl(dentry, name, buffer, size, type);
- acl = v9fs_get_cached_acl(dentry->d_inode, type);
+ acl = v9fs_get_cached_acl(inode, type);
if (IS_ERR(acl))
return PTR_ERR(acl);
if (acl == NULL)
diff --git a/fs/9p/xattr_security.c b/fs/9p/xattr_security.c
index cb247a1..df692f9 100644
--- a/fs/9p/xattr_security.c
+++ b/fs/9p/xattr_security.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_security_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_security_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_trusted.c b/fs/9p/xattr_trusted.c
index e30d33b..acfadcf 100644
--- a/fs/9p/xattr_trusted.c
+++ b/fs/9p/xattr_trusted.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_trusted_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_trusted_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_user.c b/fs/9p/xattr_user.c
index d0b701b..9b37675 100644
--- a/fs/9p/xattr_user.c
+++ b/fs/9p/xattr_user.c
@@ -19,8 +19,9 @@
#include
#include "xattr.h"
-static int v9fs_xattr_user_get(struct dentry *dentry, const char *name,
- void *buffer, size_t size, int type)
+static int v9fs_xattr_user_get(struct dentry *dentry, struct inode *inode,
+ const char *name, void *buffer, size_t size,
+ int type)
{
int retval;
char *full_name;
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index c0ebc4d..1d53da4 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -23,12 +23,12 @@ ext2_xattr_security_list(struct dentry *dentry, char *list,
size_t list_size,
[PATCH 3.18.y 4/5] switch xattr_handler->set() to passing dentry and inode separately
From: Al Viro
preparation for similar switch in ->setxattr() (see the next commit for
rationale).
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18 including ext3]
Signed-off-by: Seung-Woo Kim
---
fs/9p/acl.c |7 +++
fs/9p/xattr_security.c |5 +++--
fs/9p/xattr_trusted.c|5 +++--
fs/9p/xattr_user.c |5 +++--
fs/ext2/xattr_security.c |7 ---
fs/ext2/xattr_trusted.c |7 ---
fs/ext2/xattr_user.c |9 +
fs/ext3/xattr_security.c |7 ---
fs/ext3/xattr_trusted.c |7 ---
fs/ext3/xattr_user.c |9 +
fs/ext4/xattr_security.c |7 ---
fs/ext4/xattr_trusted.c |7 ---
fs/ext4/xattr_user.c |9 +
fs/f2fs/xattr.c | 17 -
fs/gfs2/xattr.c |7 ---
fs/hfsplus/xattr.c |7 ---
fs/hfsplus/xattr.h |4 ++--
fs/hfsplus/xattr_security.c |7 ---
fs/hfsplus/xattr_trusted.c |7 ---
fs/hfsplus/xattr_user.c |7 ---
fs/jffs2/security.c |7 ---
fs/jffs2/xattr_trusted.c |7 ---
fs/jffs2/xattr_user.c|7 ---
fs/nfs/nfs4proc.c| 19 +--
fs/ocfs2/xattr.c | 23 +--
fs/posix_acl.c |6 +++---
fs/reiserfs/xattr.c |6 --
fs/reiserfs/xattr_security.c |8
fs/reiserfs/xattr_trusted.c |8
fs/reiserfs/xattr_user.c |8
fs/xattr.c |5 +++--
fs/xfs/xfs_xattr.c |7 ---
include/linux/xattr.h|5 +++--
33 files changed, 144 insertions(+), 119 deletions(-)
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
index 938e1a4..0928d74 100644
--- a/fs/9p/acl.c
+++ b/fs/9p/acl.c
@@ -279,14 +279,13 @@ static int v9fs_remote_set_acl(struct dentry *dentry,
const char *name,
}
-static int v9fs_xattr_set_acl(struct dentry *dentry, const char *name,
- const void *value, size_t size,
- int flags, int type)
+static int v9fs_xattr_set_acl(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
struct posix_acl *acl;
struct v9fs_session_info *v9ses;
- struct inode *inode = dentry->d_inode;
if (strcmp(name, "") != 0)
return -EINVAL;
diff --git a/fs/9p/xattr_security.c b/fs/9p/xattr_security.c
index df692f9..242c1f8 100644
--- a/fs/9p/xattr_security.c
+++ b/fs/9p/xattr_security.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_security_get(struct dentry *dentry,
struct inode *inode,
return retval;
}
-static int v9fs_xattr_security_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_security_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_trusted.c b/fs/9p/xattr_trusted.c
index acfadcf..a1fe7d9 100644
--- a/fs/9p/xattr_trusted.c
+++ b/fs/9p/xattr_trusted.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_trusted_get(struct dentry *dentry,
struct inode *inode,
return retval;
}
-static int v9fs_xattr_trusted_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_trusted_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/9p/xattr_user.c b/fs/9p/xattr_user.c
index 9b37675..7a34aca 100644
--- a/fs/9p/xattr_user.c
+++ b/fs/9p/xattr_user.c
@@ -47,8 +47,9 @@ static int v9fs_xattr_user_get(struct dentry *dentry, struct
inode *inode,
return retval;
}
-static int v9fs_xattr_user_set(struct dentry *dentry, const char *name,
- const void *value, size_t size, int flags, int type)
+static int v9fs_xattr_user_set(struct dentry *dentry, struct inode *inode,
+ const char *name, const void *value,
+ size_t size, int flags, int type)
{
int retval;
char *full_name;
diff --git a/fs/ext2/xattr_security.c b/fs/ext2/xattr_security.c
index 1d53da4..3ece525 100644
--- a/fs/ext2/xattr_security.c
+++ b/fs/ext2/xattr_security.c
@@ -33,12 +33,13 @@ ext2_xattr_security_get(struct dentry *unused, struct inode
*inode,
}
static int
-ext2_xattr_security_set(struct dentry *dentry, const char *name,
-
[PATCH 3.18.y 3/5] security_d_instantiate(): move to the point prior to attaching dentry to inode
From: Al Viro
commit b96809173e94ea2fa8c19c2e40e8545a1821bf57 upstream.
Signed-off-by: Al Viro
[sw0312.kim: backport to 3.18]
Signed-off-by: Seung-Woo Kim
---
fs/dcache.c | 18 +++---
1 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/fs/dcache.c b/fs/dcache.c
index a34d401..6a4290a 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1729,12 +1729,12 @@ static void __d_instantiate(struct dentry *dentry,
struct inode *inode)
void d_instantiate(struct dentry *entry, struct inode * inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
if (inode)
spin_lock(>i_lock);
__d_instantiate(entry, inode);
if (inode)
spin_unlock(>i_lock);
- security_d_instantiate(entry, inode);
}
EXPORT_SYMBOL(d_instantiate);
@@ -1795,16 +1795,15 @@ struct dentry *d_instantiate_unique(struct dentry
*entry, struct inode *inode)
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
if (inode)
spin_lock(>i_lock);
result = __d_instantiate_unique(entry, inode);
if (inode)
spin_unlock(>i_lock);
- if (!result) {
- security_d_instantiate(entry, inode);
+ if (!result)
return NULL;
- }
BUG_ON(!d_unhashed(result));
iput(inode);
@@ -1826,6 +1825,7 @@ int d_instantiate_no_diralias(struct dentry *entry,
struct inode *inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
+ security_d_instantiate(entry, inode);
spin_lock(>i_lock);
if (S_ISDIR(inode->i_mode) && !hlist_empty(>i_dentry)) {
spin_unlock(>i_lock);
@@ -1834,7 +1834,6 @@ int d_instantiate_no_diralias(struct dentry *entry,
struct inode *inode)
}
__d_instantiate(entry, inode);
spin_unlock(>i_lock);
- security_d_instantiate(entry, inode);
return 0;
}
@@ -1930,6 +1929,7 @@ static struct dentry *__d_obtain_alias(struct inode
*inode, int disconnected)
goto out_iput;
}
+ security_d_instantiate(tmp, inode);
spin_lock(>i_lock);
res = __d_find_any_alias(inode);
if (res) {
@@ -1953,13 +1953,10 @@ static struct dentry *__d_obtain_alias(struct inode
*inode, int disconnected)
hlist_bl_unlock(>d_sb->s_anon);
spin_unlock(>d_lock);
spin_unlock(>i_lock);
- security_d_instantiate(tmp, inode);
return tmp;
out_iput:
- if (res && !IS_ERR(res))
- security_d_instantiate(res, inode);
iput(inode);
return res;
}
@@ -2817,6 +2814,7 @@ struct dentry *d_splice_alias(struct inode *inode, struct
dentry *dentry)
return ERR_CAST(inode);
if (inode && S_ISDIR(inode->i_mode)) {
+ security_d_instantiate(dentry, inode);
spin_lock(>i_lock);
new = __d_find_any_alias(inode);
if (new) {
@@ -2836,13 +2834,11 @@ struct dentry *d_splice_alias(struct inode *inode,
struct dentry *dentry)
__d_move(new, dentry, false);
write_sequnlock(_lock);
spin_unlock(>i_lock);
- security_d_instantiate(new, inode);
iput(inode);
} else {
/* already taking inode->i_lock, so d_add() by hand */
__d_instantiate(dentry, inode);
spin_unlock(>i_lock);
- security_d_instantiate(dentry, inode);
d_rehash(dentry);
}
} else {
@@ -2876,6 +2872,7 @@ struct dentry *d_materialise_unique(struct dentry
*dentry, struct inode *inode)
goto out_nolock;
}
+ security_d_instantiate(dentry, inode);
spin_lock(>i_lock);
if (S_ISDIR(inode->i_mode)) {
@@ -2927,7 +2924,6 @@ found:
spin_unlock(>i_lock);
out_nolock:
if (actual == dentry) {
- security_d_instantiate(dentry, inode);
return NULL;
}
--
1.7.4.1
Re: Linux 3.18.111
On 2018년 07월 05일 09:52, Al Viro wrote:
> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim wrote:
>>>
>>> I think the commit itself is required. Simple, but not reliable,
>>> workaround fix is like below:
>>>
>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>> index a34d401..7c751f2 100644
>>> --- a/fs/dcache.c
>>> +++ b/fs/dcache.c
>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>> struct inode *inode)
>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>> BUG_ON(!inode);
>>> lockdep_annotate_inode_mutex_key(inode);
>>> + /* WORKAROUND for calling security_d_instantiate() */
>>> + entry->d_inode = inode;
>>> security_d_instantiate(entry, inode);
>>> spin_lock(>i_lock);
>>> __d_instantiate(entry, inode);
>>
>> Ugh. That looks horrible even if it might avoid the oops.
>>
>> I think a much better solution is to back-port commit b296821a7c42
>> ("xattr_handler: pass dentry and inode as separate arguments of
>> ->get()") to older kernels. Then the inode is passed down all the way,
>> and you don't have people try to get it from the (not yet initialized)
>> dentry.
>>
>> But there might be other parts missing too, and I didn't look at how
>> easy/painful that backport would be.
>>
>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>> d_instantiate/unlock_new_inode combinations safely") being marked for
>> stable, and various cases of security_d_instantiate() calling down to
>> getxattr. Which used to not get the inode at all, so those older
>> kernels use d_inode(dentry), which doesn't work in this path since
>> dentry->d_inode hasn't been instantiated yet..
>
> You also want b96809173e94 and ce23e6401334 there...
For above two commits, also b296821a7c42 is required. And after
backport, smack still crashed because setxattr. To fix it, 5930122683df
and 3767e255b390 are also required.
By the way, does no one have met this kind getxattr crash issue with
selinux from 3.18.y?
--
Seung-Woo Kim
Samsung Research
--
Re: Linux 3.18.111
On 2018년 07월 05일 09:52, Al Viro wrote:
> On Mon, Jul 02, 2018 at 10:01:25PM -0700, Linus Torvalds wrote:
>> On Mon, Jul 2, 2018 at 9:43 PM Seung-Woo Kim wrote:
>>>
>>> I think the commit itself is required. Simple, but not reliable,
>>> workaround fix is like below:
>>>
>>> diff --git a/fs/dcache.c b/fs/dcache.c
>>> index a34d401..7c751f2 100644
>>> --- a/fs/dcache.c
>>> +++ b/fs/dcache.c
>>> @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry,
>>> struct inode *inode)
>>> BUG_ON(!hlist_unhashed(>d_u.d_alias));
>>> BUG_ON(!inode);
>>> lockdep_annotate_inode_mutex_key(inode);
>>> + /* WORKAROUND for calling security_d_instantiate() */
>>> + entry->d_inode = inode;
>>> security_d_instantiate(entry, inode);
>>> spin_lock(>i_lock);
>>> __d_instantiate(entry, inode);
>>
>> Ugh. That looks horrible even if it might avoid the oops.
>>
>> I think a much better solution is to back-port commit b296821a7c42
>> ("xattr_handler: pass dentry and inode as separate arguments of
>> ->get()") to older kernels. Then the inode is passed down all the way,
>> and you don't have people try to get it from the (not yet initialized)
>> dentry.
>>
>> But there might be other parts missing too, and I didn't look at how
>> easy/painful that backport would be.
>>
>> Al - comments? This is all because of commit 1e2e547a93a0 ("do
>> d_instantiate/unlock_new_inode combinations safely") being marked for
>> stable, and various cases of security_d_instantiate() calling down to
>> getxattr. Which used to not get the inode at all, so those older
>> kernels use d_inode(dentry), which doesn't work in this path since
>> dentry->d_inode hasn't been instantiated yet..
>
> You also want b96809173e94 and ce23e6401334 there...
For above two commits, also b296821a7c42 is required. And after
backport, smack still crashed because setxattr. To fix it, 5930122683df
and 3767e255b390 are also required.
By the way, does no one have met this kind getxattr crash issue with
selinux from 3.18.y?
--
Seung-Woo Kim
Samsung Research
--
Re: Linux 3.18.111
On 2018년 07월 03일 13:36, Greg KH wrote: > On Tue, Jul 03, 2018 at 12:24:59PM +0900, Seung-Woo Kim wrote: >> Hello, >> >> On 2018년 05월 30일 16:32, Greg KH wrote: >>> I'm announcing the release of the 3.18.111 kernel. >>> >>> All users of the 3.18 kernel series must upgrade. >>> >>> The updated 3.18.y git tree can be found at: >>> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git >>> linux-3.18.y >>> and can be browsed at the normal kernel.org git web browser: >>> >>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary >>> >>> thanks, >>> >>> greg k-h >>> >>> >> >> >> >>> do d_instantiate/unlock_new_inode combinations safely >> >> Recent my test in 3.18.113 kernel with security smack showed following >> crash during mkdir on ext4 fs. >> >> Unable to handle kernel paging request at virtual address ff98 >> pgd = ffc012411000 >> [ff98] *pgd=, *pud= >> [ cut here ] >> Kernel BUG at ffc0007d9430 [verbose debug info unavailable] >> Internal error: Oops - BUG: 9605 [#1] PREEMPT SMP >> CPU: 0 MPIDR: 8000 PID: 1237 Comm: mkdir Not tainted >> 3.18.113-00083-g1bfc02f-dirty #29-Tizen >> task: ffc02cbc2340 ti: ffc02b7fc000 task.ti: ffc02b7fc000 >> PC is at down_read+0x24/0x54 >> LR is at down_read+0x24/0x54 >> [...] >> Call trace: >> [] down_read+0x24/0x54 >> [] ext4_xattr_get+0x74/0x1f4 >> [] ext4_xattr_security_get+0x28/0x38 >> [] generic_getxattr+0x4c/0x60 >> [] smk_fetch.isra.6+0x8c/0xe0 >> [] smack_d_instantiate+0x194/0x324 >> [] security_d_instantiate+0x24/0x30 >> [] d_instantiate_new+0x34/0x94 >> [] ext4_mkdir+0x284/0x354 >> [] vfs_mkdir+0xc0/0x150 >> [] SyS_mkdirat+0x88/0xb8 >> [] SyS_mkdir+0x18/0x20 >> Code: aa0003f3 b00017c0 912e1000 97e38943 (c85f7e60) >> ---[ end trace b1ad797d63dae9c5 ]--- >> >> It is because d_instantiate_new() added from above commit calls >> security_d_instantiate() before calling __d_instantiate() and >> dentry->d_inode is not yet set and null. In 3.18.113 kernel, >> inode->i_op_getxattr() of ext4 is still generic_getxattr() and it only >> has dentry parameter without inode, so it tries to access dentry->d_inode. >> >> I did not test with selinux, but selinux also calls >> inode->i_op_getxattr() from selinux_d_instantiate(), so maybe there is >> also same issue. > > So should I revert something or do you have a proposed fix for this? I think the commit itself is required. Simple, but not reliable, workaround fix is like below: diff --git a/fs/dcache.c b/fs/dcache.c index a34d401..7c751f2 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry, struct inode *inode) BUG_ON(!hlist_unhashed(>d_u.d_alias)); BUG_ON(!inode); lockdep_annotate_inode_mutex_key(inode); + /* WORKAROUND for calling security_d_instantiate() */ + entry->d_inode = inode; security_d_instantiate(entry, inode); spin_lock(>i_lock); __d_instantiate(entry, inode); --- But I am not familiar with dentry/inode locking and there is no lock consideration at all. Thanks, - Seung-Woo Kim > > thanks, > > greg k-h > > > -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.111
On 2018년 07월 03일 13:36, Greg KH wrote: > On Tue, Jul 03, 2018 at 12:24:59PM +0900, Seung-Woo Kim wrote: >> Hello, >> >> On 2018년 05월 30일 16:32, Greg KH wrote: >>> I'm announcing the release of the 3.18.111 kernel. >>> >>> All users of the 3.18 kernel series must upgrade. >>> >>> The updated 3.18.y git tree can be found at: >>> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git >>> linux-3.18.y >>> and can be browsed at the normal kernel.org git web browser: >>> >>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary >>> >>> thanks, >>> >>> greg k-h >>> >>> >> >> >> >>> do d_instantiate/unlock_new_inode combinations safely >> >> Recent my test in 3.18.113 kernel with security smack showed following >> crash during mkdir on ext4 fs. >> >> Unable to handle kernel paging request at virtual address ff98 >> pgd = ffc012411000 >> [ff98] *pgd=, *pud= >> [ cut here ] >> Kernel BUG at ffc0007d9430 [verbose debug info unavailable] >> Internal error: Oops - BUG: 9605 [#1] PREEMPT SMP >> CPU: 0 MPIDR: 8000 PID: 1237 Comm: mkdir Not tainted >> 3.18.113-00083-g1bfc02f-dirty #29-Tizen >> task: ffc02cbc2340 ti: ffc02b7fc000 task.ti: ffc02b7fc000 >> PC is at down_read+0x24/0x54 >> LR is at down_read+0x24/0x54 >> [...] >> Call trace: >> [] down_read+0x24/0x54 >> [] ext4_xattr_get+0x74/0x1f4 >> [] ext4_xattr_security_get+0x28/0x38 >> [] generic_getxattr+0x4c/0x60 >> [] smk_fetch.isra.6+0x8c/0xe0 >> [] smack_d_instantiate+0x194/0x324 >> [] security_d_instantiate+0x24/0x30 >> [] d_instantiate_new+0x34/0x94 >> [] ext4_mkdir+0x284/0x354 >> [] vfs_mkdir+0xc0/0x150 >> [] SyS_mkdirat+0x88/0xb8 >> [] SyS_mkdir+0x18/0x20 >> Code: aa0003f3 b00017c0 912e1000 97e38943 (c85f7e60) >> ---[ end trace b1ad797d63dae9c5 ]--- >> >> It is because d_instantiate_new() added from above commit calls >> security_d_instantiate() before calling __d_instantiate() and >> dentry->d_inode is not yet set and null. In 3.18.113 kernel, >> inode->i_op_getxattr() of ext4 is still generic_getxattr() and it only >> has dentry parameter without inode, so it tries to access dentry->d_inode. >> >> I did not test with selinux, but selinux also calls >> inode->i_op_getxattr() from selinux_d_instantiate(), so maybe there is >> also same issue. > > So should I revert something or do you have a proposed fix for this? I think the commit itself is required. Simple, but not reliable, workaround fix is like below: diff --git a/fs/dcache.c b/fs/dcache.c index a34d401..7c751f2 100644 --- a/fs/dcache.c +++ b/fs/dcache.c @@ -1879,6 +1879,8 @@ void d_instantiate_new(struct dentry *entry, struct inode *inode) BUG_ON(!hlist_unhashed(>d_u.d_alias)); BUG_ON(!inode); lockdep_annotate_inode_mutex_key(inode); + /* WORKAROUND for calling security_d_instantiate() */ + entry->d_inode = inode; security_d_instantiate(entry, inode); spin_lock(>i_lock); __d_instantiate(entry, inode); --- But I am not familiar with dentry/inode locking and there is no lock consideration at all. Thanks, - Seung-Woo Kim > > thanks, > > greg k-h > > > -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.111
Hello, On 2018년 05월 30일 16:32, Greg KH wrote: > I'm announcing the release of the 3.18.111 kernel. > > All users of the 3.18 kernel series must upgrade. > > The updated 3.18.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-3.18.y > and can be browsed at the normal kernel.org git web browser: > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > thanks, > > greg k-h > > > do d_instantiate/unlock_new_inode combinations safely Recent my test in 3.18.113 kernel with security smack showed following crash during mkdir on ext4 fs. Unable to handle kernel paging request at virtual address ff98 pgd = ffc012411000 [ff98] *pgd=, *pud= [ cut here ] Kernel BUG at ffc0007d9430 [verbose debug info unavailable] Internal error: Oops - BUG: 9605 [#1] PREEMPT SMP CPU: 0 MPIDR: 8000 PID: 1237 Comm: mkdir Not tainted 3.18.113-00083-g1bfc02f-dirty #29-Tizen task: ffc02cbc2340 ti: ffc02b7fc000 task.ti: ffc02b7fc000 PC is at down_read+0x24/0x54 LR is at down_read+0x24/0x54 [...] Call trace: [] down_read+0x24/0x54 [] ext4_xattr_get+0x74/0x1f4 [] ext4_xattr_security_get+0x28/0x38 [] generic_getxattr+0x4c/0x60 [] smk_fetch.isra.6+0x8c/0xe0 [] smack_d_instantiate+0x194/0x324 [] security_d_instantiate+0x24/0x30 [] d_instantiate_new+0x34/0x94 [] ext4_mkdir+0x284/0x354 [] vfs_mkdir+0xc0/0x150 [] SyS_mkdirat+0x88/0xb8 [] SyS_mkdir+0x18/0x20 Code: aa0003f3 b00017c0 912e1000 97e38943 (c85f7e60) ---[ end trace b1ad797d63dae9c5 ]--- It is because d_instantiate_new() added from above commit calls security_d_instantiate() before calling __d_instantiate() and dentry->d_inode is not yet set and null. In 3.18.113 kernel, inode->i_op_getxattr() of ext4 is still generic_getxattr() and it only has dentry parameter without inode, so it tries to access dentry->d_inode. I did not test with selinux, but selinux also calls inode->i_op_getxattr() from selinux_d_instantiate(), so maybe there is also same issue. Best Regards, - Seung-Woo Kim -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.111
Hello, On 2018년 05월 30일 16:32, Greg KH wrote: > I'm announcing the release of the 3.18.111 kernel. > > All users of the 3.18 kernel series must upgrade. > > The updated 3.18.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-3.18.y > and can be browsed at the normal kernel.org git web browser: > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > thanks, > > greg k-h > > > do d_instantiate/unlock_new_inode combinations safely Recent my test in 3.18.113 kernel with security smack showed following crash during mkdir on ext4 fs. Unable to handle kernel paging request at virtual address ff98 pgd = ffc012411000 [ff98] *pgd=, *pud= [ cut here ] Kernel BUG at ffc0007d9430 [verbose debug info unavailable] Internal error: Oops - BUG: 9605 [#1] PREEMPT SMP CPU: 0 MPIDR: 8000 PID: 1237 Comm: mkdir Not tainted 3.18.113-00083-g1bfc02f-dirty #29-Tizen task: ffc02cbc2340 ti: ffc02b7fc000 task.ti: ffc02b7fc000 PC is at down_read+0x24/0x54 LR is at down_read+0x24/0x54 [...] Call trace: [] down_read+0x24/0x54 [] ext4_xattr_get+0x74/0x1f4 [] ext4_xattr_security_get+0x28/0x38 [] generic_getxattr+0x4c/0x60 [] smk_fetch.isra.6+0x8c/0xe0 [] smack_d_instantiate+0x194/0x324 [] security_d_instantiate+0x24/0x30 [] d_instantiate_new+0x34/0x94 [] ext4_mkdir+0x284/0x354 [] vfs_mkdir+0xc0/0x150 [] SyS_mkdirat+0x88/0xb8 [] SyS_mkdir+0x18/0x20 Code: aa0003f3 b00017c0 912e1000 97e38943 (c85f7e60) ---[ end trace b1ad797d63dae9c5 ]--- It is because d_instantiate_new() added from above commit calls security_d_instantiate() before calling __d_instantiate() and dentry->d_inode is not yet set and null. In 3.18.113 kernel, inode->i_op_getxattr() of ext4 is still generic_getxattr() and it only has dentry parameter without inode, so it tries to access dentry->d_inode. I did not test with selinux, but selinux also calls inode->i_op_getxattr() from selinux_d_instantiate(), so maybe there is also same issue. Best Regards, - Seung-Woo Kim -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.102
On 2018년 03월 27일 16:05, Greg KH wrote: > On Tue, Mar 27, 2018 at 10:40:33AM +0900, Seung-Woo Kim wrote: >> Hello, >> >> Until 3.18.102, it looks like following patch series for v4l2 seems missed. >> >> 273caa2 media: v4l2-compat-ioctl32.c: make ctrl_is_pointer work for subdevs >> a1dfb4c media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic >> d83a824 media: v4l2-compat-ioctl32.c: don't copy back the result for >> certain errors >> 169f24c media: v4l2-compat-ioctl32.c: drop pr_info for unknown buffer type >> a751be5 media: v4l2-compat-ioctl32.c: copy clip list in put_v4l2_window32 >> b8c601e media: v4l2-compat-ioctl32.c: fix ctrl_is_pointer >> 8ed5a59 media: v4l2-compat-ioctl32.c: copy m.userptr in put_v4l2_plane32 >> 333b1e9 media: v4l2-compat-ioctl32.c: avoid sizeof(type) >> 486c521 media: v4l2-compat-ioctl32.c: move 'helper' functions to >> __get/put_v4l2_format32 >> b7b957d media: v4l2-compat-ioctl32.c: fix the indentation >> 3ee6d04 media: v4l2-compat-ioctl32.c: add missing VIDIOC_PREPARE_BUF >> 181a4a2 media: v4l2-ioctl.c: don't copy back the result for -ENOTTY > > That is correct. > >> Any plan to apply the series? > > Do you have a backported version of this series that will apply to that > kernel tree? If so, I'll be glad to apply it but I'm pretty sure the > reason I did not apply them was because they did not apply correctly. I dit not do it myself. I just checked the series is applied on other stable trees. > > Also, are you even sure you need these patches for 3.18.y? Do you have > a v4l2 device that relies on these fixes? I've just expected A-34624167 is also fixed from other ARM64 devices based on 3.18.y. Best Regards, - Seung-Woo Kim > > thanks, > > greg k-h > > > -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.102
On 2018년 03월 27일 16:05, Greg KH wrote: > On Tue, Mar 27, 2018 at 10:40:33AM +0900, Seung-Woo Kim wrote: >> Hello, >> >> Until 3.18.102, it looks like following patch series for v4l2 seems missed. >> >> 273caa2 media: v4l2-compat-ioctl32.c: make ctrl_is_pointer work for subdevs >> a1dfb4c media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic >> d83a824 media: v4l2-compat-ioctl32.c: don't copy back the result for >> certain errors >> 169f24c media: v4l2-compat-ioctl32.c: drop pr_info for unknown buffer type >> a751be5 media: v4l2-compat-ioctl32.c: copy clip list in put_v4l2_window32 >> b8c601e media: v4l2-compat-ioctl32.c: fix ctrl_is_pointer >> 8ed5a59 media: v4l2-compat-ioctl32.c: copy m.userptr in put_v4l2_plane32 >> 333b1e9 media: v4l2-compat-ioctl32.c: avoid sizeof(type) >> 486c521 media: v4l2-compat-ioctl32.c: move 'helper' functions to >> __get/put_v4l2_format32 >> b7b957d media: v4l2-compat-ioctl32.c: fix the indentation >> 3ee6d04 media: v4l2-compat-ioctl32.c: add missing VIDIOC_PREPARE_BUF >> 181a4a2 media: v4l2-ioctl.c: don't copy back the result for -ENOTTY > > That is correct. > >> Any plan to apply the series? > > Do you have a backported version of this series that will apply to that > kernel tree? If so, I'll be glad to apply it but I'm pretty sure the > reason I did not apply them was because they did not apply correctly. I dit not do it myself. I just checked the series is applied on other stable trees. > > Also, are you even sure you need these patches for 3.18.y? Do you have > a v4l2 device that relies on these fixes? I've just expected A-34624167 is also fixed from other ARM64 devices based on 3.18.y. Best Regards, - Seung-Woo Kim > > thanks, > > greg k-h > > > -- Seung-Woo Kim Samsung Research --
Re: Linux 3.18.102
Hello, Until 3.18.102, it looks like following patch series for v4l2 seems missed. 273caa2 media: v4l2-compat-ioctl32.c: make ctrl_is_pointer work for subdevs a1dfb4c media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic d83a824 media: v4l2-compat-ioctl32.c: don't copy back the result for certain errors 169f24c media: v4l2-compat-ioctl32.c: drop pr_info for unknown buffer type a751be5 media: v4l2-compat-ioctl32.c: copy clip list in put_v4l2_window32 b8c601e media: v4l2-compat-ioctl32.c: fix ctrl_is_pointer 8ed5a59 media: v4l2-compat-ioctl32.c: copy m.userptr in put_v4l2_plane32 333b1e9 media: v4l2-compat-ioctl32.c: avoid sizeof(type) 486c521 media: v4l2-compat-ioctl32.c: move 'helper' functions to __get/put_v4l2_format32 b7b957d media: v4l2-compat-ioctl32.c: fix the indentation 3ee6d04 media: v4l2-compat-ioctl32.c: add missing VIDIOC_PREPARE_BUF 181a4a2 media: v4l2-ioctl.c: don't copy back the result for -ENOTTY Any plan to apply the series? Best Regards, - Seung-Woo Kim On 2018년 03월 25일 17:59, Greg KH wrote: > I'm announcing the release of the 3.18.102 kernel. > > All users of the 3.18 kernel series must upgrade. > > The updated 3.18.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-3.18.y > and can be browsed at the normal kernel.org git web browser: > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > thanks, > > greg k-h >
Re: Linux 3.18.102
Hello, Until 3.18.102, it looks like following patch series for v4l2 seems missed. 273caa2 media: v4l2-compat-ioctl32.c: make ctrl_is_pointer work for subdevs a1dfb4c media: v4l2-compat-ioctl32.c: refactor compat ioctl32 logic d83a824 media: v4l2-compat-ioctl32.c: don't copy back the result for certain errors 169f24c media: v4l2-compat-ioctl32.c: drop pr_info for unknown buffer type a751be5 media: v4l2-compat-ioctl32.c: copy clip list in put_v4l2_window32 b8c601e media: v4l2-compat-ioctl32.c: fix ctrl_is_pointer 8ed5a59 media: v4l2-compat-ioctl32.c: copy m.userptr in put_v4l2_plane32 333b1e9 media: v4l2-compat-ioctl32.c: avoid sizeof(type) 486c521 media: v4l2-compat-ioctl32.c: move 'helper' functions to __get/put_v4l2_format32 b7b957d media: v4l2-compat-ioctl32.c: fix the indentation 3ee6d04 media: v4l2-compat-ioctl32.c: add missing VIDIOC_PREPARE_BUF 181a4a2 media: v4l2-ioctl.c: don't copy back the result for -ENOTTY Any plan to apply the series? Best Regards, - Seung-Woo Kim On 2018년 03월 25일 17:59, Greg KH wrote: > I'm announcing the release of the 3.18.102 kernel. > > All users of the 3.18 kernel series must upgrade. > > The updated 3.18.y git tree can be found at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git > linux-3.18.y > and can be browsed at the normal kernel.org git web browser: > > http://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=summary > > thanks, > > greg k-h >
[PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
Select ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration to enable UBSAN on arm. Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |2 ++ arch/arm/vdso/Makefile|2 ++ 3 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 7888c98..5eb3c89 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -11,6 +11,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..8f06ab2 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -24,6 +24,8 @@ endif GCOV_PROFILE := n +UBSAN_SANITIZE := n + # # Architecture dependencies # diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..e343700 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -29,6 +29,8 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n + # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.4.1
[PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
Select ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration to enable UBSAN on arm. Signed-off-by: Seung-Woo Kim --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |2 ++ arch/arm/vdso/Makefile|2 ++ 3 files changed, 5 insertions(+), 0 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 7888c98..5eb3c89 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -11,6 +11,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT if CPU_V7 diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..8f06ab2 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -24,6 +24,8 @@ endif GCOV_PROFILE := n +UBSAN_SANITIZE := n + # # Architecture dependencies # diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..e343700 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -29,6 +29,8 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n + # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.4.1
[RESEND][PATCH] Kbuild: fix file name in comment about extra gcc checks
Extra gcc checks like W=1 were moved to scripts/Makefile.exrawarn, so the file name in comment needs to be fixed. Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 165cf97..faa9d26 100644 --- a/Makefile +++ b/Makefile @@ -707,7 +707,7 @@ KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) else # These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.build) +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) endif -- 1.7.4.1
[RESEND][PATCH] Kbuild: fix file name in comment about extra gcc checks
Extra gcc checks like W=1 were moved to scripts/Makefile.exrawarn, so the file name in comment needs to be fixed. Signed-off-by: Seung-Woo Kim --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 165cf97..faa9d26 100644 --- a/Makefile +++ b/Makefile @@ -707,7 +707,7 @@ KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) else # These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.build) +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) endif -- 1.7.4.1
[PATCH] Kbuild: fix file name in comment about extra gcc checks
Extra gcc checks like W=1 were moved to scripts/Makefile.exrawarn, so the file name in comment needs to be fixed. Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 0988400..dbaada3 100644 --- a/Makefile +++ b/Makefile @@ -705,7 +705,7 @@ KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) else # These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.build) +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) endif -- 1.7.4.1
[PATCH] Kbuild: fix file name in comment about extra gcc checks
Extra gcc checks like W=1 were moved to scripts/Makefile.exrawarn, so the file name in comment needs to be fixed. Signed-off-by: Seung-Woo Kim --- Makefile |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/Makefile b/Makefile index 0988400..dbaada3 100644 --- a/Makefile +++ b/Makefile @@ -705,7 +705,7 @@ KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior) else # These warnings generated too much noise in a regular build. -# Use make W=1 to enable them (see scripts/Makefile.build) +# Use make W=1 to enable them (see scripts/Makefile.extrawarn) KBUILD_CFLAGS += $(call cc-disable-warning, unused-but-set-variable) KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable) endif -- 1.7.4.1
[PATCH] Smack: ignore private inode for file functions
The access to fd from anon_inode is always failed because there is
no set xattr operations. So this patch fixes to ignore private
inode including anon_inode for file functions.
It was only ignored for smack_file_receive() to share dma-buf fd,
but dma-buf has other functions like ioctl and mmap.
Reference: https://lkml.org/lkml/2015/4/17/16
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
security/smack/smack_lsm.c | 12
1 file changed, 12 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 1cb0602..e7f0bbe 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1632,6 +1632,9 @@ static int smack_file_ioctl(struct file *file, unsigned
int cmd,
struct smk_audit_info ad;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH);
smk_ad_setfield_u_fs_path(, file->f_path);
@@ -1661,6 +1664,9 @@ static int smack_file_lock(struct file *file, unsigned
int cmd)
int rc;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH);
smk_ad_setfield_u_fs_path(, file->f_path);
rc = smk_curacc(smk_of_inode(inode), MAY_LOCK, );
@@ -1687,6 +1693,9 @@ static int smack_file_fcntl(struct file *file, unsigned
int cmd,
int rc = 0;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
switch (cmd) {
case F_GETLK:
break;
@@ -1740,6 +1749,9 @@ static int smack_mmap_file(struct file *file,
if (file == NULL)
return 0;
+ if (unlikely(IS_PRIVATE(file_inode(file
+ return 0;
+
isp = file_inode(file)->i_security;
if (isp->smk_mmap == NULL)
return 0;
--
1.7.9.5
[PATCH] Smack: ignore private inode for file functions
The access to fd from anon_inode is always failed because there is
no set xattr operations. So this patch fixes to ignore private
inode including anon_inode for file functions.
It was only ignored for smack_file_receive() to share dma-buf fd,
but dma-buf has other functions like ioctl and mmap.
Reference: https://lkml.org/lkml/2015/4/17/16
Signed-off-by: Seung-Woo Kim
---
security/smack/smack_lsm.c | 12
1 file changed, 12 insertions(+)
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 1cb0602..e7f0bbe 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1632,6 +1632,9 @@ static int smack_file_ioctl(struct file *file, unsigned
int cmd,
struct smk_audit_info ad;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH);
smk_ad_setfield_u_fs_path(, file->f_path);
@@ -1661,6 +1664,9 @@ static int smack_file_lock(struct file *file, unsigned
int cmd)
int rc;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH);
smk_ad_setfield_u_fs_path(, file->f_path);
rc = smk_curacc(smk_of_inode(inode), MAY_LOCK, );
@@ -1687,6 +1693,9 @@ static int smack_file_fcntl(struct file *file, unsigned
int cmd,
int rc = 0;
struct inode *inode = file_inode(file);
+ if (unlikely(IS_PRIVATE(inode)))
+ return 0;
+
switch (cmd) {
case F_GETLK:
break;
@@ -1740,6 +1749,9 @@ static int smack_mmap_file(struct file *file,
if (file == NULL)
return 0;
+ if (unlikely(IS_PRIVATE(file_inode(file
+ return 0;
+
isp = file_inode(file)->i_security;
if (isp->smk_mmap == NULL)
return 0;
--
1.7.9.5
Re: [PATCH v2] arm64: dts: exynos: Add flash led dt node for TM2 board
Hello Chanwoo,
On 2016년 11월 30일 17:36, Chanwoo Choi wrote:
> Dear Seung-Woo,
>
> I think that this patch looks good to me.
> But, When I tested this patch on my TM2 board,
> the flash turn off after some millisecond automatically.
Thank you for testing on your side. I will check on my side about the
issue, and if I find, then I will send next version.
Regards,
- Seung-Woo Kim
>
> It is strange situation. Unfortunately, I don't know the cause.
> I think that we better to check this issue for more time.
>
> Best Regards,
> Chanwoo Choi
>
> On 2016년 11월 30일 13:48, Seung-Woo Kim wrote:
>> From: Ingi Kim <ingi2@samsung.com>
>>
>> This patch adds Kinetic ktd2692 flash led device node for TM2 board.
>>
>> Signed-off-by: Ingi Kim <ingi2@samsung.com>
>> Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
>> ---
>> Change from v1:
>> - gpio active value is set with defined macro instead of value.
>> ---
>> arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
>> 1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index f21bdc2..0d454aa 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -58,6 +58,19 @@
>> reg = <0x0 0x2000 0x0 0xc000>;
>> };
>>
>> +camera-flash {
>> +compatible = "kinetic,ktd2692";
>> +ctrl-gpios = < 1 GPIO_ACTIVE_HIGH>;
>> +aux-gpios = < 2 GPIO_ACTIVE_HIGH>;
>> +
>> +flash-led {
>> +label = "ktd2692-flash";
>> + led-max-microamp = <30>;
>> +flash-max-microamp = <150>;
>> +flash-max-timeout-us = <1835000>;
>> +};
>> +};
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>>
>>
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH v2] arm64: dts: exynos: Add flash led dt node for TM2 board
Hello Chanwoo,
On 2016년 11월 30일 17:36, Chanwoo Choi wrote:
> Dear Seung-Woo,
>
> I think that this patch looks good to me.
> But, When I tested this patch on my TM2 board,
> the flash turn off after some millisecond automatically.
Thank you for testing on your side. I will check on my side about the
issue, and if I find, then I will send next version.
Regards,
- Seung-Woo Kim
>
> It is strange situation. Unfortunately, I don't know the cause.
> I think that we better to check this issue for more time.
>
> Best Regards,
> Chanwoo Choi
>
> On 2016년 11월 30일 13:48, Seung-Woo Kim wrote:
>> From: Ingi Kim
>>
>> This patch adds Kinetic ktd2692 flash led device node for TM2 board.
>>
>> Signed-off-by: Ingi Kim
>> Signed-off-by: Seung-Woo Kim
>> ---
>> Change from v1:
>> - gpio active value is set with defined macro instead of value.
>> ---
>> arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
>> 1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> index f21bdc2..0d454aa 100644
>> --- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> +++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
>> @@ -58,6 +58,19 @@
>> reg = <0x0 0x2000 0x0 0xc000>;
>> };
>>
>> +camera-flash {
>> +compatible = "kinetic,ktd2692";
>> +ctrl-gpios = < 1 GPIO_ACTIVE_HIGH>;
>> +aux-gpios = < 2 GPIO_ACTIVE_HIGH>;
>> +
>> +flash-led {
>> +label = "ktd2692-flash";
>> +led-max-microamp = <30>;
>> +flash-max-microamp = <150>;
>> +flash-max-timeout-us = <1835000>;
>> +};
>> +};
>> +
>> gpio-keys {
>> compatible = "gpio-keys";
>>
>>
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
[PATCH v2] arm64: dts: exynos: Add flash led dt node for TM2 board
From: Ingi Kim <ingi2@samsung.com>
This patch adds Kinetic ktd2692 flash led device node for TM2 board.
Signed-off-by: Ingi Kim <ingi2@samsung.com>
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
Change from v1:
- gpio active value is set with defined macro instead of value.
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..0d454aa 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -58,6 +58,19 @@
reg = <0x0 0x2000 0x0 0xc000>;
};
+ camera-flash {
+ compatible = "kinetic,ktd2692";
+ ctrl-gpios = < 1 GPIO_ACTIVE_HIGH>;
+ aux-gpios = < 2 GPIO_ACTIVE_HIGH>;
+
+ flash-led {
+ label = "ktd2692-flash";
+ led-max-microamp = <30>;
+ flash-max-microamp = <150>;
+ flash-max-timeout-us = <1835000>;
+ };
+ };
+
gpio-keys {
compatible = "gpio-keys";
--
1.7.4.1
[PATCH v2] arm64: dts: exynos: Add flash led dt node for TM2 board
From: Ingi Kim
This patch adds Kinetic ktd2692 flash led device node for TM2 board.
Signed-off-by: Ingi Kim
Signed-off-by: Seung-Woo Kim
---
Change from v1:
- gpio active value is set with defined macro instead of value.
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..0d454aa 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -58,6 +58,19 @@
reg = <0x0 0x2000 0x0 0xc000>;
};
+ camera-flash {
+ compatible = "kinetic,ktd2692";
+ ctrl-gpios = < 1 GPIO_ACTIVE_HIGH>;
+ aux-gpios = < 2 GPIO_ACTIVE_HIGH>;
+
+ flash-led {
+ label = "ktd2692-flash";
+ led-max-microamp = <30>;
+ flash-max-microamp = <150>;
+ flash-max-timeout-us = <1835000>;
+ };
+ };
+
gpio-keys {
compatible = "gpio-keys";
--
1.7.4.1
[PATCH] arm64: dts: exynos: Add flash led dt node for TM2 board
From: Ingi Kim <ingi2@samsung.com>
This patch adds Kinetic ktd2692 flash led device node for TM2 board.
Signed-off-by: Ingi Kim <ingi2@samsung.com>
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..3ba305f 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -58,6 +58,19 @@
reg = <0x0 0x2000 0x0 0xc000>;
};
+ camera-flash {
+ compatible = "kinetic,ktd2692";
+ ctrl-gpios = < 1 0>;
+ aux-gpios = < 2 0>;
+
+ flash-led {
+ label = "ktd2692-flash";
+ led-max-microamp = <30>;
+ flash-max-microamp = <150>;
+ flash-max-timeout-us = <1835000>;
+ };
+ };
+
gpio-keys {
compatible = "gpio-keys";
--
1.7.4.1
[PATCH] arm64: dts: exynos: Add flash led dt node for TM2 board
From: Ingi Kim
This patch adds Kinetic ktd2692 flash led device node for TM2 board.
Signed-off-by: Ingi Kim
Signed-off-by: Seung-Woo Kim
---
arch/arm64/boot/dts/exynos/exynos5433-tm2.dts | 13 +
1 files changed, 13 insertions(+), 0 deletions(-)
diff --git a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
index f21bdc2..3ba305f 100644
--- a/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
+++ b/arch/arm64/boot/dts/exynos/exynos5433-tm2.dts
@@ -58,6 +58,19 @@
reg = <0x0 0x2000 0x0 0xc000>;
};
+ camera-flash {
+ compatible = "kinetic,ktd2692";
+ ctrl-gpios = < 1 0>;
+ aux-gpios = < 2 0>;
+
+ flash-led {
+ label = "ktd2692-flash";
+ led-max-microamp = <30>;
+ flash-max-microamp = <150>;
+ flash-max-timeout-us = <1835000>;
+ };
+ };
+
gpio-keys {
compatible = "gpio-keys";
--
1.7.4.1
UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h
With the patch "arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL", I got following UBSAN warning on Exynos5422 SoC board. UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h:296:17 negation of -2147483648 cannot be represented in type 'int': CPU: 2 PID: 5637 Comm: fsck.ext4 Tainted: GW 4.8.0-rc7-00173-g142932f #25 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (ubsan_epilogue+0x14/0x50) [] (ubsan_epilogue) from [] (__ubsan_handle_negate_overflow+0x68/0x6c) [] (__ubsan_handle_negate_overflow) from [] (radix_tree_next_chunk+0x4e8/0x5ac) [] (radix_tree_next_chunk) from [] (find_get_pages_tag+0x1b0/0x390) [] (find_get_pages_tag) from [] (pagevec_lookup_tag+0x38/0x68) [] (pagevec_lookup_tag) from [] (write_cache_pages+0x134/0x85c) [] (write_cache_pages) from [] (generic_writepages+0x5c/0x9c) [] (generic_writepages) from [] (blkdev_writepages+0x18/0x1c) [] (blkdev_writepages) from [] (do_writepages+0x58/0xa4) [] (do_writepages) from [] (__filemap_fdatawrite_range+0xac/0x10c) [] (__filemap_fdatawrite_range) from [] (filemap_write_and_wait_range+0x54/0xb8) [] (filemap_write_and_wait_range) from [] (blkdev_fsync+0x40/0xa4) [] (blkdev_fsync) from [] (vfs_fsync_range+0x70/0x160) [] (vfs_fsync_range) from [] (do_fsync+0x4c/0x74) [] (do_fsync) from [] (SyS_fsync+0x1c/0x20) [] (SyS_fsync) from [] (ret_fast_syscall+0x0/0x3c) UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h:296:17 negation of -2147483648 cannot be represented in type 'int': CPU: 3 PID: 6313 Comm: enlightenment Tainted: GW 4.8.0-rc7-00173-g142932f #25 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (ubsan_epilogue+0x14/0x50) [] (ubsan_epilogue) from [] (__ubsan_handle_negate_overflow+0x68/0x6c) [] (__ubsan_handle_negate_overflow) from [] (next_signal+0x114/0x16c) [] (next_signal) from [] (dequeue_signal+0x30/0x30c) [] (dequeue_signal) from [] (get_signal+0x120/0xe84) [] (get_signal) from [] (do_signal+0x37c/0x550) [] (do_signal) from [] (do_work_pending+0xd0/0x134) [] (do_work_pending) from [] (slow_work_pending+0xc/0x20) Regards, - Seung-Woo Kim -- Seung-Woo Kim Samsung Software R Center --
UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h
With the patch "arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL", I got following UBSAN warning on Exynos5422 SoC board. UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h:296:17 negation of -2147483648 cannot be represented in type 'int': CPU: 2 PID: 5637 Comm: fsck.ext4 Tainted: GW 4.8.0-rc7-00173-g142932f #25 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (ubsan_epilogue+0x14/0x50) [] (ubsan_epilogue) from [] (__ubsan_handle_negate_overflow+0x68/0x6c) [] (__ubsan_handle_negate_overflow) from [] (radix_tree_next_chunk+0x4e8/0x5ac) [] (radix_tree_next_chunk) from [] (find_get_pages_tag+0x1b0/0x390) [] (find_get_pages_tag) from [] (pagevec_lookup_tag+0x38/0x68) [] (pagevec_lookup_tag) from [] (write_cache_pages+0x134/0x85c) [] (write_cache_pages) from [] (generic_writepages+0x5c/0x9c) [] (generic_writepages) from [] (blkdev_writepages+0x18/0x1c) [] (blkdev_writepages) from [] (do_writepages+0x58/0xa4) [] (do_writepages) from [] (__filemap_fdatawrite_range+0xac/0x10c) [] (__filemap_fdatawrite_range) from [] (filemap_write_and_wait_range+0x54/0xb8) [] (filemap_write_and_wait_range) from [] (blkdev_fsync+0x40/0xa4) [] (blkdev_fsync) from [] (vfs_fsync_range+0x70/0x160) [] (vfs_fsync_range) from [] (do_fsync+0x4c/0x74) [] (do_fsync) from [] (SyS_fsync+0x1c/0x20) [] (SyS_fsync) from [] (ret_fast_syscall+0x0/0x3c) UBSAN: Undefined behaviour in ./arch/arm/include/asm/bitops.h:296:17 negation of -2147483648 cannot be represented in type 'int': CPU: 3 PID: 6313 Comm: enlightenment Tainted: GW 4.8.0-rc7-00173-g142932f #25 Hardware name: SAMSUNG EXYNOS (Flattened Device Tree) [] (unwind_backtrace) from [] (show_stack+0x20/0x24) [] (show_stack) from [] (dump_stack+0x84/0xa0) [] (dump_stack) from [] (ubsan_epilogue+0x14/0x50) [] (ubsan_epilogue) from [] (__ubsan_handle_negate_overflow+0x68/0x6c) [] (__ubsan_handle_negate_overflow) from [] (next_signal+0x114/0x16c) [] (next_signal) from [] (dequeue_signal+0x30/0x30c) [] (dequeue_signal) from [] (get_signal+0x120/0xe84) [] (get_signal) from [] (do_signal+0x37c/0x550) [] (do_signal) from [] (do_work_pending+0xd0/0x134) [] (do_work_pending) from [] (slow_work_pending+0xc/0x20) Regards, - Seung-Woo Kim -- Seung-Woo Kim Samsung Software R Center --
[PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
To enable UBSAN on arm, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration. Basic kernel bootup test is passed on arm with CONFIG_UBSAN_SANITIZE_ALL enabled. Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- This is resend of the patch I already sent, [1], without RFC tag. [1] https://patchwork.kernel.org/patch/9189533/ I tested kernel build and basic boot up on Exynos5422, Exynos4412 and Exynos3250 SoC boards. At previous time on [1], there were some build error on other systems, but they were caused by driver bug or gcc bug. So I think UBSAN on ARM can be re-considered. --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |1 + arch/arm/vdso/Makefile|1 + 3 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index a9c4e48..a80f9b1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -7,6 +7,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_USE_BUILTIN_BSWAP diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..883374f 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -23,6 +23,7 @@ OBJS += hyp-stub.o endif GCOV_PROFILE := n +UBSAN_SANITIZE := n # # Architecture dependencies diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..cb90e59 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -28,6 +28,7 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.4.1
[PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
To enable UBSAN on arm, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration. Basic kernel bootup test is passed on arm with CONFIG_UBSAN_SANITIZE_ALL enabled. Signed-off-by: Seung-Woo Kim --- This is resend of the patch I already sent, [1], without RFC tag. [1] https://patchwork.kernel.org/patch/9189533/ I tested kernel build and basic boot up on Exynos5422, Exynos4412 and Exynos3250 SoC boards. At previous time on [1], there were some build error on other systems, but they were caused by driver bug or gcc bug. So I think UBSAN on ARM can be re-considered. --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |1 + arch/arm/vdso/Makefile|1 + 3 files changed, 3 insertions(+), 0 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index a9c4e48..a80f9b1 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -7,6 +7,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_USE_BUILTIN_BSWAP diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..883374f 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -23,6 +23,7 @@ OBJS += hyp-stub.o endif GCOV_PROFILE := n +UBSAN_SANITIZE := n # # Architecture dependencies diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..cb90e59 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -28,6 +28,7 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.4.1
[PATCH v2] pwm: samsung: fix to use lowest div for large enough modulation bits
>From pwm_samsung_calc_tin(), there is routine to find the lowest
divider possible to generate lower frequency than requested one.
But it is always possible to generate requested frequency with
large enough modulation bits except s3c24xx, so this patch fixes
to use lowest div for the case. This patch removes following UBSAN
warning:
UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
shift exponent 32 is too large for 32-bit type 'long unsigned int'
[...]
[] (ubsan_epilogue) from []
(__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
[] (__ubsan_handle_shift_out_of_bounds) from []
(pwm_samsung_config+0x508/0x6a4)
[] (pwm_samsung_config) from []
(pwm_apply_state+0x174/0x40c)
[] (pwm_apply_state) from [] (pwm_fan_probe+0xc8/0x488)
[] (pwm_fan_probe) from []
(platform_drv_probe+0x70/0x150)
[...]
Cc: Tomasz Figa <tomasz.f...@gmail.com>
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
Changes from v1
* Remove too complex condition, instead, just checking simple variant->bits as
Krzysztof's suggestion
* Add comment from Tomasz
The UBSAN warning from ARM is reported with the patch in following link:
https://patchwork.kernel.org/patch/9189575/
---
drivers/pwm/pwm-samsung.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index ada2d32..f113cda 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -193,9 +193,18 @@ static unsigned long pwm_samsung_calc_tin(struct
samsung_pwm_chip *chip,
* divider settings and choose the lowest divisor that can generate
* frequencies lower than requested.
*/
- for (div = variant->div_base; div < 4; ++div)
- if ((rate >> (variant->bits + div)) < freq)
- break;
+ if (variant->bits < 32) {
+ /* Only for s3c24xx */
+ for (div = variant->div_base; div < 4; ++div)
+ if ((rate >> (variant->bits + div)) < freq)
+ break;
+ } else {
+ /*
+* Other variants have enough counter bits to generate any
+* requested rate, so no need to check higher divisors.
+*/
+ div = variant->div_base;
+ }
pwm_samsung_set_divisor(chip, chan, BIT(div));
--
2.9.2
[PATCH v2] pwm: samsung: fix to use lowest div for large enough modulation bits
>From pwm_samsung_calc_tin(), there is routine to find the lowest
divider possible to generate lower frequency than requested one.
But it is always possible to generate requested frequency with
large enough modulation bits except s3c24xx, so this patch fixes
to use lowest div for the case. This patch removes following UBSAN
warning:
UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
shift exponent 32 is too large for 32-bit type 'long unsigned int'
[...]
[] (ubsan_epilogue) from []
(__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
[] (__ubsan_handle_shift_out_of_bounds) from []
(pwm_samsung_config+0x508/0x6a4)
[] (pwm_samsung_config) from []
(pwm_apply_state+0x174/0x40c)
[] (pwm_apply_state) from [] (pwm_fan_probe+0xc8/0x488)
[] (pwm_fan_probe) from []
(platform_drv_probe+0x70/0x150)
[...]
Cc: Tomasz Figa
Signed-off-by: Seung-Woo Kim
---
Changes from v1
* Remove too complex condition, instead, just checking simple variant->bits as
Krzysztof's suggestion
* Add comment from Tomasz
The UBSAN warning from ARM is reported with the patch in following link:
https://patchwork.kernel.org/patch/9189575/
---
drivers/pwm/pwm-samsung.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index ada2d32..f113cda 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -193,9 +193,18 @@ static unsigned long pwm_samsung_calc_tin(struct
samsung_pwm_chip *chip,
* divider settings and choose the lowest divisor that can generate
* frequencies lower than requested.
*/
- for (div = variant->div_base; div < 4; ++div)
- if ((rate >> (variant->bits + div)) < freq)
- break;
+ if (variant->bits < 32) {
+ /* Only for s3c24xx */
+ for (div = variant->div_base; div < 4; ++div)
+ if ((rate >> (variant->bits + div)) < freq)
+ break;
+ } else {
+ /*
+* Other variants have enough counter bits to generate any
+* requested rate, so no need to check higher divisors.
+*/
+ div = variant->div_base;
+ }
pwm_samsung_set_divisor(chip, chan, BIT(div));
--
2.9.2
Re: [PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
Hello Krzysztof,
On 2016년 08월 16일 18:10, Krzysztof Kozlowski wrote:
> On 08/16/2016 11:00 AM, Tomasz Figa wrote:
>> 2016-08-16 17:25 GMT+09:00 Seung-Woo Kim <sw0312@samsung.com>:
>>> Hi Krzysztof,
>>>
>>> On 2016년 08월 16일 16:37, Krzysztof Kozlowski wrote:
>>>> On 08/02/2016 12:16 PM, Seung-Woo Kim wrote:
>>>>> >From pwm_samsung_calc_tin(), there is routine to find the lowest
>>>>> divider possible to generate lower frequency than requested one.
>>>>> But it is always possible to generate requested frequency with
>>>>> large enough modulation bits, so this patch fixes to use lowest
>>>>> div for the case. This patch removes following UBSAN warning:
>>>>>
>>>>>UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
>>>>>shift exponent 32 is too large for 32-bit type 'long unsigned int'
>>>>>[...]
>>>>>[] (ubsan_epilogue) from []
>>>>> (__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
>>>>>[] (__ubsan_handle_shift_out_of_bounds) from []
>>>>> (pwm_samsung_config+0x508/0x6a4)
>>>>> [] (pwm_samsung_config) from []
>>>>> (pwm_apply_state+0x174/0x40c)
>>>>>[] (pwm_apply_state) from []
>>>>> (pwm_fan_probe+0xc8/0x488)
>>>>>[] (pwm_fan_probe) from []
>>>>> (platform_drv_probe+0x70/0x150)
>>>>>[...]
>>>>>
>>>>> Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
>>>>> ---
>>>>> The UBSAN warning from ARM is reported with the patch in following link:
>>>>> https://patchwork.kernel.org/patch/9189575/
>>>>> ---
>>>>> drivers/pwm/pwm-samsung.c | 10 +++---
>>>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
>>>>> index ada2d32..ff0def6 100644
>>>>> --- a/drivers/pwm/pwm-samsung.c
>>>>> +++ b/drivers/pwm/pwm-samsung.c
>>>>> @@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
>>>>> samsung_pwm_chip *chip,
>>>>> * divider settings and choose the lowest divisor that can generate
>>>>> * frequencies lower than requested.
>>>>> */
>>>>> -for (div = variant->div_base; div < 4; ++div)
>>>>> -if ((rate >> (variant->bits + div)) < freq)
>>>>> -break;
>>>>> +if (fls(rate) <= variant->bits) {
>>>>> +div = variant->div_base;
>>>>> +} else {
>>>>> +for (div = variant->div_base; div < 4; ++div)
>>>>> +if ((rate >> (variant->bits + div)) < freq)
>>>>> +break;
>>>>> +}
>>>>
>>>> I have trouble with understanding the idea behind initial code from
>>>> Tomasz (commit 11ad39ede24ee). The variant->bits for all SoC except
>>>> S3C24xx is 32. This means the shift:
>>>> if ((rate >> (variant->bits + div)) < freq)
>>>> will be always by 32 or more... In practice this will choose always a
>>>> "div" of 0 because in first iteration of this loop, the shift will be by
>>>> 32.
>>>
>>> I also confused that part, but I figured out that the bit is used to
>>> consider modulation bit to generate pwm signal from the input clock.
>>>
>>> Only the old s3c2440 has 16 bit modulation timer for pwm, and all later
>>> soc has 32 bit modulation timer. So 32 bit timer cases, with the lowest
>>> div, it can generate all frequencies which can be assigned with 32bit
>>> variable.
>>> But I uses fls() to consider 64bit case also even though there is no
>>> really that kind of clock.
>>
>> The code may look complicated (in fact I had to think a bit to recall
>> what exactly it was supposed to do), but I'm not sure how it could be
>> simplified. It's generally intended to handle variant->bits < 32 cases
>> only and is effectively a no-op when variant->bits >= 32.
>
> Right, a comment for this behavior would be very useful. No need to
> waste time for re-thinking it later.
>
>> I would suggest just making rate an u64 and be done with the warning.
>> IMHO adding this kind of special cases only complicates the (already
>> complicated) code unnecessarily.
>
> u64 could solve the warning but then one would have to figure out
> whether the casts are safe or not. Unsigned long is assigned to rate and
> then returned.
>
> How about specific check (+comment) like:
> if (variant->bits < 32) {
> /* Only for s3c24xx */
> // the for loop as it was
> } else {
> /* For other variants just choose lowest divider always */
> div = variant->div_base;
> }
>
> For me this is quite obvious and error-prone (explicit check for value
> to be used in shift).
Actually above was my internal first version and it also removes UBSAN
warning. I will send as you suggested with Tomasz's comment.
Thanks,
- Seung-Woo Kim
>
> Best regards,
> Krzysztof
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
Hello Krzysztof,
On 2016년 08월 16일 18:10, Krzysztof Kozlowski wrote:
> On 08/16/2016 11:00 AM, Tomasz Figa wrote:
>> 2016-08-16 17:25 GMT+09:00 Seung-Woo Kim :
>>> Hi Krzysztof,
>>>
>>> On 2016년 08월 16일 16:37, Krzysztof Kozlowski wrote:
>>>> On 08/02/2016 12:16 PM, Seung-Woo Kim wrote:
>>>>> >From pwm_samsung_calc_tin(), there is routine to find the lowest
>>>>> divider possible to generate lower frequency than requested one.
>>>>> But it is always possible to generate requested frequency with
>>>>> large enough modulation bits, so this patch fixes to use lowest
>>>>> div for the case. This patch removes following UBSAN warning:
>>>>>
>>>>>UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
>>>>>shift exponent 32 is too large for 32-bit type 'long unsigned int'
>>>>>[...]
>>>>>[] (ubsan_epilogue) from []
>>>>> (__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
>>>>>[] (__ubsan_handle_shift_out_of_bounds) from []
>>>>> (pwm_samsung_config+0x508/0x6a4)
>>>>> [] (pwm_samsung_config) from []
>>>>> (pwm_apply_state+0x174/0x40c)
>>>>>[] (pwm_apply_state) from []
>>>>> (pwm_fan_probe+0xc8/0x488)
>>>>>[] (pwm_fan_probe) from []
>>>>> (platform_drv_probe+0x70/0x150)
>>>>>[...]
>>>>>
>>>>> Signed-off-by: Seung-Woo Kim
>>>>> ---
>>>>> The UBSAN warning from ARM is reported with the patch in following link:
>>>>> https://patchwork.kernel.org/patch/9189575/
>>>>> ---
>>>>> drivers/pwm/pwm-samsung.c | 10 +++---
>>>>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
>>>>> index ada2d32..ff0def6 100644
>>>>> --- a/drivers/pwm/pwm-samsung.c
>>>>> +++ b/drivers/pwm/pwm-samsung.c
>>>>> @@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
>>>>> samsung_pwm_chip *chip,
>>>>> * divider settings and choose the lowest divisor that can generate
>>>>> * frequencies lower than requested.
>>>>> */
>>>>> -for (div = variant->div_base; div < 4; ++div)
>>>>> -if ((rate >> (variant->bits + div)) < freq)
>>>>> -break;
>>>>> +if (fls(rate) <= variant->bits) {
>>>>> +div = variant->div_base;
>>>>> +} else {
>>>>> +for (div = variant->div_base; div < 4; ++div)
>>>>> +if ((rate >> (variant->bits + div)) < freq)
>>>>> +break;
>>>>> +}
>>>>
>>>> I have trouble with understanding the idea behind initial code from
>>>> Tomasz (commit 11ad39ede24ee). The variant->bits for all SoC except
>>>> S3C24xx is 32. This means the shift:
>>>> if ((rate >> (variant->bits + div)) < freq)
>>>> will be always by 32 or more... In practice this will choose always a
>>>> "div" of 0 because in first iteration of this loop, the shift will be by
>>>> 32.
>>>
>>> I also confused that part, but I figured out that the bit is used to
>>> consider modulation bit to generate pwm signal from the input clock.
>>>
>>> Only the old s3c2440 has 16 bit modulation timer for pwm, and all later
>>> soc has 32 bit modulation timer. So 32 bit timer cases, with the lowest
>>> div, it can generate all frequencies which can be assigned with 32bit
>>> variable.
>>> But I uses fls() to consider 64bit case also even though there is no
>>> really that kind of clock.
>>
>> The code may look complicated (in fact I had to think a bit to recall
>> what exactly it was supposed to do), but I'm not sure how it could be
>> simplified. It's generally intended to handle variant->bits < 32 cases
>> only and is effectively a no-op when variant->bits >= 32.
>
> Right, a comment for this behavior would be very useful. No need to
> waste time for re-thinking it later.
>
>> I would suggest just making rate an u64 and be done with the warning.
>> IMHO adding this kind of special cases only complicates the (already
>> complicated) code unnecessarily.
>
> u64 could solve the warning but then one would have to figure out
> whether the casts are safe or not. Unsigned long is assigned to rate and
> then returned.
>
> How about specific check (+comment) like:
> if (variant->bits < 32) {
> /* Only for s3c24xx */
> // the for loop as it was
> } else {
> /* For other variants just choose lowest divider always */
> div = variant->div_base;
> }
>
> For me this is quite obvious and error-prone (explicit check for value
> to be used in shift).
Actually above was my internal first version and it also removes UBSAN
warning. I will send as you suggested with Tomasz's comment.
Thanks,
- Seung-Woo Kim
>
> Best regards,
> Krzysztof
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
Hi Krzysztof,
On 2016년 08월 16일 16:37, Krzysztof Kozlowski wrote:
> On 08/02/2016 12:16 PM, Seung-Woo Kim wrote:
>> >From pwm_samsung_calc_tin(), there is routine to find the lowest
>> divider possible to generate lower frequency than requested one.
>> But it is always possible to generate requested frequency with
>> large enough modulation bits, so this patch fixes to use lowest
>> div for the case. This patch removes following UBSAN warning:
>>
>>UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
>>shift exponent 32 is too large for 32-bit type 'long unsigned int'
>>[...]
>>[] (ubsan_epilogue) from []
>> (__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
>>[] (__ubsan_handle_shift_out_of_bounds) from []
>> (pwm_samsung_config+0x508/0x6a4)
>>[] (pwm_samsung_config) from []
>> (pwm_apply_state+0x174/0x40c)
>>[] (pwm_apply_state) from []
>> (pwm_fan_probe+0xc8/0x488)
>>[] (pwm_fan_probe) from []
>> (platform_drv_probe+0x70/0x150)
>>[...]
>>
>> Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
>> ---
>> The UBSAN warning from ARM is reported with the patch in following link:
>> https://patchwork.kernel.org/patch/9189575/
>> ---
>> drivers/pwm/pwm-samsung.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
>> index ada2d32..ff0def6 100644
>> --- a/drivers/pwm/pwm-samsung.c
>> +++ b/drivers/pwm/pwm-samsung.c
>> @@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
>> samsung_pwm_chip *chip,
>> * divider settings and choose the lowest divisor that can generate
>> * frequencies lower than requested.
>> */
>> -for (div = variant->div_base; div < 4; ++div)
>> -if ((rate >> (variant->bits + div)) < freq)
>> -break;
>> +if (fls(rate) <= variant->bits) {
>> +div = variant->div_base;
>> +} else {
>> +for (div = variant->div_base; div < 4; ++div)
>> +if ((rate >> (variant->bits + div)) < freq)
>> +break;
>> +}
>
> I have trouble with understanding the idea behind initial code from
> Tomasz (commit 11ad39ede24ee). The variant->bits for all SoC except
> S3C24xx is 32. This means the shift:
> if ((rate >> (variant->bits + div)) < freq)
> will be always by 32 or more... In practice this will choose always a
> "div" of 0 because in first iteration of this loop, the shift will be by 32.
I also confused that part, but I figured out that the bit is used to
consider modulation bit to generate pwm signal from the input clock.
Only the old s3c2440 has 16 bit modulation timer for pwm, and all later
soc has 32 bit modulation timer. So 32 bit timer cases, with the lowest
div, it can generate all frequencies which can be assigned with 32bit
variable.
But I uses fls() to consider 64bit case also even though there is no
really that kind of clock.
Best Regards,
- Seung-Woo Kim
>
> This looks weird and the fix does not really remove the weirdness out of it.
>
> Best regards,
> Krzysztof
>
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
Hi Krzysztof,
On 2016년 08월 16일 16:37, Krzysztof Kozlowski wrote:
> On 08/02/2016 12:16 PM, Seung-Woo Kim wrote:
>> >From pwm_samsung_calc_tin(), there is routine to find the lowest
>> divider possible to generate lower frequency than requested one.
>> But it is always possible to generate requested frequency with
>> large enough modulation bits, so this patch fixes to use lowest
>> div for the case. This patch removes following UBSAN warning:
>>
>>UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
>>shift exponent 32 is too large for 32-bit type 'long unsigned int'
>>[...]
>>[] (ubsan_epilogue) from []
>> (__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
>>[] (__ubsan_handle_shift_out_of_bounds) from []
>> (pwm_samsung_config+0x508/0x6a4)
>>[] (pwm_samsung_config) from []
>> (pwm_apply_state+0x174/0x40c)
>>[] (pwm_apply_state) from []
>> (pwm_fan_probe+0xc8/0x488)
>>[] (pwm_fan_probe) from []
>> (platform_drv_probe+0x70/0x150)
>>[...]
>>
>> Signed-off-by: Seung-Woo Kim
>> ---
>> The UBSAN warning from ARM is reported with the patch in following link:
>> https://patchwork.kernel.org/patch/9189575/
>> ---
>> drivers/pwm/pwm-samsung.c | 10 +++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
>> index ada2d32..ff0def6 100644
>> --- a/drivers/pwm/pwm-samsung.c
>> +++ b/drivers/pwm/pwm-samsung.c
>> @@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
>> samsung_pwm_chip *chip,
>> * divider settings and choose the lowest divisor that can generate
>> * frequencies lower than requested.
>> */
>> -for (div = variant->div_base; div < 4; ++div)
>> -if ((rate >> (variant->bits + div)) < freq)
>> -break;
>> +if (fls(rate) <= variant->bits) {
>> +div = variant->div_base;
>> +} else {
>> +for (div = variant->div_base; div < 4; ++div)
>> +if ((rate >> (variant->bits + div)) < freq)
>> +break;
>> +}
>
> I have trouble with understanding the idea behind initial code from
> Tomasz (commit 11ad39ede24ee). The variant->bits for all SoC except
> S3C24xx is 32. This means the shift:
> if ((rate >> (variant->bits + div)) < freq)
> will be always by 32 or more... In practice this will choose always a
> "div" of 0 because in first iteration of this loop, the shift will be by 32.
I also confused that part, but I figured out that the bit is used to
consider modulation bit to generate pwm signal from the input clock.
Only the old s3c2440 has 16 bit modulation timer for pwm, and all later
soc has 32 bit modulation timer. So 32 bit timer cases, with the lowest
div, it can generate all frequencies which can be assigned with 32bit
variable.
But I uses fls() to consider 64bit case also even though there is no
really that kind of clock.
Best Regards,
- Seung-Woo Kim
>
> This looks weird and the fix does not really remove the weirdness out of it.
>
> Best regards,
> Krzysztof
>
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
[PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
>From pwm_samsung_calc_tin(), there is routine to find the lowest
divider possible to generate lower frequency than requested one.
But it is always possible to generate requested frequency with
large enough modulation bits, so this patch fixes to use lowest
div for the case. This patch removes following UBSAN warning:
UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
shift exponent 32 is too large for 32-bit type 'long unsigned int'
[...]
[] (ubsan_epilogue) from []
(__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
[] (__ubsan_handle_shift_out_of_bounds) from []
(pwm_samsung_config+0x508/0x6a4)
[] (pwm_samsung_config) from []
(pwm_apply_state+0x174/0x40c)
[] (pwm_apply_state) from [] (pwm_fan_probe+0xc8/0x488)
[] (pwm_fan_probe) from []
(platform_drv_probe+0x70/0x150)
[...]
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
The UBSAN warning from ARM is reported with the patch in following link:
https://patchwork.kernel.org/patch/9189575/
---
drivers/pwm/pwm-samsung.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index ada2d32..ff0def6 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
samsung_pwm_chip *chip,
* divider settings and choose the lowest divisor that can generate
* frequencies lower than requested.
*/
- for (div = variant->div_base; div < 4; ++div)
- if ((rate >> (variant->bits + div)) < freq)
- break;
+ if (fls(rate) <= variant->bits) {
+ div = variant->div_base;
+ } else {
+ for (div = variant->div_base; div < 4; ++div)
+ if ((rate >> (variant->bits + div)) < freq)
+ break;
+ }
pwm_samsung_set_divisor(chip, chan, BIT(div));
--
1.7.9.5
[PATCH] pwm: samsung: fix to use lowest div for large enough modulation bits
>From pwm_samsung_calc_tin(), there is routine to find the lowest
divider possible to generate lower frequency than requested one.
But it is always possible to generate requested frequency with
large enough modulation bits, so this patch fixes to use lowest
div for the case. This patch removes following UBSAN warning:
UBSAN: Undefined behaviour in drivers/pwm/pwm-samsung.c:197:13
shift exponent 32 is too large for 32-bit type 'long unsigned int'
[...]
[] (ubsan_epilogue) from []
(__ubsan_handle_shift_out_of_bounds+0xd8/0x120)
[] (__ubsan_handle_shift_out_of_bounds) from []
(pwm_samsung_config+0x508/0x6a4)
[] (pwm_samsung_config) from []
(pwm_apply_state+0x174/0x40c)
[] (pwm_apply_state) from [] (pwm_fan_probe+0xc8/0x488)
[] (pwm_fan_probe) from []
(platform_drv_probe+0x70/0x150)
[...]
Signed-off-by: Seung-Woo Kim
---
The UBSAN warning from ARM is reported with the patch in following link:
https://patchwork.kernel.org/patch/9189575/
---
drivers/pwm/pwm-samsung.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/pwm/pwm-samsung.c b/drivers/pwm/pwm-samsung.c
index ada2d32..ff0def6 100644
--- a/drivers/pwm/pwm-samsung.c
+++ b/drivers/pwm/pwm-samsung.c
@@ -193,9 +193,13 @@ static unsigned long pwm_samsung_calc_tin(struct
samsung_pwm_chip *chip,
* divider settings and choose the lowest divisor that can generate
* frequencies lower than requested.
*/
- for (div = variant->div_base; div < 4; ++div)
- if ((rate >> (variant->bits + div)) < freq)
- break;
+ if (fls(rate) <= variant->bits) {
+ div = variant->div_base;
+ } else {
+ for (div = variant->div_base; div < 4; ++div)
+ if ((rate >> (variant->bits + div)) < freq)
+ break;
+ }
pwm_samsung_set_divisor(chip, chan, BIT(div));
--
1.7.9.5
[PATCH] memory: samsung: exynos-srom: fix wrong count of registers
This patch fixes wrong count of array for srom registers from probe
function.
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
drivers/memory/samsung/exynos-srom.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/memory/samsung/exynos-srom.c
b/drivers/memory/samsung/exynos-srom.c
index 96756fb..c66d2bd 100644
--- a/drivers/memory/samsung/exynos-srom.c
+++ b/drivers/memory/samsung/exynos-srom.c
@@ -134,7 +134,7 @@ static int exynos_srom_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, srom);
srom->reg_offset = exynos_srom_alloc_reg_dump(exynos_srom_offsets,
- sizeof(exynos_srom_offsets));
+ ARRAY_SIZE(exynos_srom_offsets));
if (!srom->reg_offset) {
iounmap(srom->reg_base);
return -ENOMEM;
--
1.7.9.5
[PATCH] memory: samsung: exynos-srom: fix wrong count of registers
This patch fixes wrong count of array for srom registers from probe
function.
Signed-off-by: Seung-Woo Kim
---
drivers/memory/samsung/exynos-srom.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/memory/samsung/exynos-srom.c
b/drivers/memory/samsung/exynos-srom.c
index 96756fb..c66d2bd 100644
--- a/drivers/memory/samsung/exynos-srom.c
+++ b/drivers/memory/samsung/exynos-srom.c
@@ -134,7 +134,7 @@ static int exynos_srom_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, srom);
srom->reg_offset = exynos_srom_alloc_reg_dump(exynos_srom_offsets,
- sizeof(exynos_srom_offsets));
+ ARRAY_SIZE(exynos_srom_offsets));
if (!srom->reg_offset) {
iounmap(srom->reg_base);
return -ENOMEM;
--
1.7.9.5
[RFC][PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
To enable UBSAN on arm, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration. Basic kernel booting is tested on arm kernel enabled CONFIG_UBSAN_SANITIZE_ALL from Exynos5422 based Odroid-XU3 board. Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- Because I tested only with specific soc board, so I am not sure the ubsan is fine for all other cases. So, I send this patch as a RFC. --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |1 + arch/arm/vdso/Makefile|1 + 3 files changed, 3 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 90542db..3a9af80 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -7,6 +7,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_USE_BUILTIN_BSWAP diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..883374f 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -23,6 +23,7 @@ OBJS += hyp-stub.o endif GCOV_PROFILE := n +UBSAN_SANITIZE := n # # Architecture dependencies diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..cb90e59 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -28,6 +28,7 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.9.5
[RFC][PATCH] arm: ubsan: select ARCH_HAS_UBSAN_SANITIZE_ALL
To enable UBSAN on arm, this patch enables ARCH_HAS_UBSAN_SANITIZE_ALL from arm confiuration. Basic kernel booting is tested on arm kernel enabled CONFIG_UBSAN_SANITIZE_ALL from Exynos5422 based Odroid-XU3 board. Signed-off-by: Seung-Woo Kim --- Because I tested only with specific soc board, so I am not sure the ubsan is fine for all other cases. So, I send this patch as a RFC. --- arch/arm/Kconfig |1 + arch/arm/boot/compressed/Makefile |1 + arch/arm/vdso/Makefile|1 + 3 files changed, 3 insertions(+) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 90542db..3a9af80 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -7,6 +7,7 @@ config ARM select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST select ARCH_HAVE_CUSTOM_GPIO_H select ARCH_HAS_GCOV_PROFILE_ALL + select ARCH_HAS_UBSAN_SANITIZE_ALL select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_SUPPORTS_ATOMIC_RMW select ARCH_USE_BUILTIN_BSWAP diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile index d50430c..883374f 100644 --- a/arch/arm/boot/compressed/Makefile +++ b/arch/arm/boot/compressed/Makefile @@ -23,6 +23,7 @@ OBJS += hyp-stub.o endif GCOV_PROFILE := n +UBSAN_SANITIZE := n # # Architecture dependencies diff --git a/arch/arm/vdso/Makefile b/arch/arm/vdso/Makefile index 59a8fa7..cb90e59 100644 --- a/arch/arm/vdso/Makefile +++ b/arch/arm/vdso/Makefile @@ -28,6 +28,7 @@ CFLAGS_vgettimeofday.o = -O2 # Disable gcov profiling for VDSO code GCOV_PROFILE := n +UBSAN_SANITIZE := n # Force dependency $(obj)/vdso.o : $(obj)/vdso.so -- 1.7.9.5
RE: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
Hello Shawn,
> -Original Message-
> From: Shawn Lin [mailto:shawn@rock-chips.com]
> Sent: Tuesday, June 21, 2016 10:52 AM
> To: Seung-Woo Kim; jh80.ch...@samsung.com; ulf.hans...@linaro.org;
> linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: shawn@rock-chips.com
> Subject: Re: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
>
> On 2016/6/20 16:34, Seung-Woo Kim wrote:
> > Hi folks,
> >
> > During booting test on my Exynos5422 based Odroid-XU3, kernel compiled
> > with CONFIG_DMA_API_DEBUG reported following warning:
> >
> > [ cut here ]
> > WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1096 check_unmap+0x7bc/0xb38
> > dwmmc_exynos 1220.mmc: DMA-API: device driver tries to free DMA memory
> > it has not allocated [device
> address=0x6d9d2200]
>
> Thanks for this report and fix.
>
> DTO(the same as IDMAC-RI/TI) interrupts may or may not come together
> with DATA_ERR. If DATA_ERR occur without geting DTO, we should issue
> CMD12 manually to generate DTO. It's a ugly deisgn for dwmmc but from
> the vendor's ask.
>
> So you should never think we complete the xfer without
> checking DATA_ERR. This way you got the warning.
>
> So could you try this one:
With your patch, there is no more the DMA API waring in my environment.
Best Regards,
- Seung-Woo Kim
>
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2474,7 +2474,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id)
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
>
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete((void *)host);
> + if (!test_bit(EVENT_DATA_ERROR,
> >pending_events))
> + host->dma_ops->complete((void *)host);
> }
> } else {
> pending = mci_readl(host, IDSTS);
> @@ -2482,7 +2483,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id)
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
>
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete((void *)host);
> + if (!test_bit(EVENT_DATA_ERROR,
> >pending_events))
> + host->dma_ops->complete((void *)host);
> }
> }
>
>
> > [size=128 bytes]
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc4 #26
> > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> > [] (unwind_backtrace) from [] (show_stack+0x20/0x24)
> > [] (show_stack) from [] (dump_stack+0x80/0x94)
> > [] (dump_stack) from [] (__warn+0xf8/0x110)
> > [] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
> > [] (warn_slowpath_fmt) from [] (check_unmap+0x7bc/0xb38)
> > [] (check_unmap) from []
> > (debug_dma_unmap_sg+0x118/0x148)
> > [] (debug_dma_unmap_sg) from []
> > (dw_mci_dma_cleanup+0x7c/0xb8)
> > [] (dw_mci_dma_cleanup) from []
> > (dw_mci_stop_dma+0x40/0x50)
> > [] (dw_mci_stop_dma) from []
> > (dw_mci_tasklet_func+0x130/0x3b4)
> > [] (dw_mci_tasklet_func) from []
> > (tasklet_action+0xb4/0x150)
> > [] (tasklet_action) from [] (__do_softirq+0xe4/0x3cc)
> > [] (__do_softirq) from [] (irq_exit+0xd0/0x10c)
> > [] (irq_exit) from [] (__handle_domain_irq+0x90/0xfc)
> > [] (__handle_domain_irq) from []
> > (gic_handle_irq+0x64/0xa8)
> > [] (gic_handle_irq) from [] (__irq_svc+0x54/0x90)
> > Exception stack(0xc1101ef8 to 0xc1101f40)
> > 1ee0: 0001
> >
> > 1f00: c011b600 c110 c110753c c11c3984 c11074d4
> > c1107548
> > 1f20: c1101f54 c1101f58 c1101f48 c010a1fc c010a200 6013
> >
> > [] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c)
> > [] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c)
> > [] (default_idle_call) from []
> > (cpu_startup_entry+0x358/0x3b4)
> > [] (cpu_startup_entry) from [] (rest_init+0x94/0x98)
> > [] (rest_init) from [] (start_kernel+0x3a4/0x3b0)
> > [] (start_kernel) from [<4000807c>] (0x4000807c)
> > ---[ end trace 256f83eed365daf0 ]---
> >
> > The warning occurs because after complete callback function,
> > dw_mci_dmac_complete_dma() is called, then dw_mci_stop_dma() is called
> > again. So it causes dma_unmap_sg() is called twice for same sg. It
&g
RE: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
Hello Shawn,
> -Original Message-
> From: Shawn Lin [mailto:shawn@rock-chips.com]
> Sent: Tuesday, June 21, 2016 10:52 AM
> To: Seung-Woo Kim; jh80.ch...@samsung.com; ulf.hans...@linaro.org;
> linux-...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Cc: shawn@rock-chips.com
> Subject: Re: mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
>
> On 2016/6/20 16:34, Seung-Woo Kim wrote:
> > Hi folks,
> >
> > During booting test on my Exynos5422 based Odroid-XU3, kernel compiled
> > with CONFIG_DMA_API_DEBUG reported following warning:
> >
> > [ cut here ]
> > WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1096 check_unmap+0x7bc/0xb38
> > dwmmc_exynos 1220.mmc: DMA-API: device driver tries to free DMA memory
> > it has not allocated [device
> address=0x6d9d2200]
>
> Thanks for this report and fix.
>
> DTO(the same as IDMAC-RI/TI) interrupts may or may not come together
> with DATA_ERR. If DATA_ERR occur without geting DTO, we should issue
> CMD12 manually to generate DTO. It's a ugly deisgn for dwmmc but from
> the vendor's ask.
>
> So you should never think we complete the xfer without
> checking DATA_ERR. This way you got the warning.
>
> So could you try this one:
With your patch, there is no more the DMA API waring in my environment.
Best Regards,
- Seung-Woo Kim
>
> --- a/drivers/mmc/host/dw_mmc.c
> +++ b/drivers/mmc/host/dw_mmc.c
> @@ -2474,7 +2474,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id)
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_TI |
>
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS64, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete((void *)host);
> + if (!test_bit(EVENT_DATA_ERROR,
> >pending_events))
> + host->dma_ops->complete((void *)host);
> }
> } else {
> pending = mci_readl(host, IDSTS);
> @@ -2482,7 +2483,8 @@ static irqreturn_t dw_mci_interrupt(int irq, void
> *dev_id)
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_TI |
>
> SDMMC_IDMAC_INT_RI);
> mci_writel(host, IDSTS, SDMMC_IDMAC_INT_NI);
> - host->dma_ops->complete((void *)host);
> + if (!test_bit(EVENT_DATA_ERROR,
> >pending_events))
> + host->dma_ops->complete((void *)host);
> }
> }
>
>
> > [size=128 bytes]
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc4 #26
> > Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
> > [] (unwind_backtrace) from [] (show_stack+0x20/0x24)
> > [] (show_stack) from [] (dump_stack+0x80/0x94)
> > [] (dump_stack) from [] (__warn+0xf8/0x110)
> > [] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
> > [] (warn_slowpath_fmt) from [] (check_unmap+0x7bc/0xb38)
> > [] (check_unmap) from []
> > (debug_dma_unmap_sg+0x118/0x148)
> > [] (debug_dma_unmap_sg) from []
> > (dw_mci_dma_cleanup+0x7c/0xb8)
> > [] (dw_mci_dma_cleanup) from []
> > (dw_mci_stop_dma+0x40/0x50)
> > [] (dw_mci_stop_dma) from []
> > (dw_mci_tasklet_func+0x130/0x3b4)
> > [] (dw_mci_tasklet_func) from []
> > (tasklet_action+0xb4/0x150)
> > [] (tasklet_action) from [] (__do_softirq+0xe4/0x3cc)
> > [] (__do_softirq) from [] (irq_exit+0xd0/0x10c)
> > [] (irq_exit) from [] (__handle_domain_irq+0x90/0xfc)
> > [] (__handle_domain_irq) from []
> > (gic_handle_irq+0x64/0xa8)
> > [] (gic_handle_irq) from [] (__irq_svc+0x54/0x90)
> > Exception stack(0xc1101ef8 to 0xc1101f40)
> > 1ee0: 0001
> >
> > 1f00: c011b600 c110 c110753c c11c3984 c11074d4
> > c1107548
> > 1f20: c1101f54 c1101f58 c1101f48 c010a1fc c010a200 6013
> >
> > [] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c)
> > [] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c)
> > [] (default_idle_call) from []
> > (cpu_startup_entry+0x358/0x3b4)
> > [] (cpu_startup_entry) from [] (rest_init+0x94/0x98)
> > [] (rest_init) from [] (start_kernel+0x3a4/0x3b0)
> > [] (start_kernel) from [<4000807c>] (0x4000807c)
> > ---[ end trace 256f83eed365daf0 ]---
> >
> > The warning occurs because after complete callback function,
> > dw_mci_dmac_complete_dma() is called, then dw_mci_stop_dma() is called
> > again. So it causes dma_unmap_sg() is called twice for same sg. It
&g
mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
Hi folks,
During booting test on my Exynos5422 based Odroid-XU3, kernel compiled
with CONFIG_DMA_API_DEBUG reported following warning:
[ cut here ]
WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1096 check_unmap+0x7bc/0xb38
dwmmc_exynos 1220.mmc: DMA-API: device driver tries to free DMA memory it
has not allocated [device address=0x6d9d2200]
[size=128 bytes]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc4 #26
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x20/0x24)
[] (show_stack) from [] (dump_stack+0x80/0x94)
[] (dump_stack) from [] (__warn+0xf8/0x110)
[] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
[] (warn_slowpath_fmt) from [] (check_unmap+0x7bc/0xb38)
[] (check_unmap) from [] (debug_dma_unmap_sg+0x118/0x148)
[] (debug_dma_unmap_sg) from []
(dw_mci_dma_cleanup+0x7c/0xb8)
[] (dw_mci_dma_cleanup) from [] (dw_mci_stop_dma+0x40/0x50)
[] (dw_mci_stop_dma) from []
(dw_mci_tasklet_func+0x130/0x3b4)
[] (dw_mci_tasklet_func) from [] (tasklet_action+0xb4/0x150)
[] (tasklet_action) from [] (__do_softirq+0xe4/0x3cc)
[] (__do_softirq) from [] (irq_exit+0xd0/0x10c)
[] (irq_exit) from [] (__handle_domain_irq+0x90/0xfc)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x64/0xa8)
[] (gic_handle_irq) from [] (__irq_svc+0x54/0x90)
Exception stack(0xc1101ef8 to 0xc1101f40)
1ee0: 0001
1f00: c011b600 c110 c110753c c11c3984 c11074d4 c1107548
1f20: c1101f54 c1101f58 c1101f48 c010a1fc c010a200 6013
[] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c)
[] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c)
[] (default_idle_call) from []
(cpu_startup_entry+0x358/0x3b4)
[] (cpu_startup_entry) from [] (rest_init+0x94/0x98)
[] (rest_init) from [] (start_kernel+0x3a4/0x3b0)
[] (start_kernel) from [<4000807c>] (0x4000807c)
---[ end trace 256f83eed365daf0 ]---
The warning occurs because after complete callback function,
dw_mci_dmac_complete_dma() is called, then dw_mci_stop_dma() is called
again. So it causes dma_unmap_sg() is called twice for same sg. It
occurs during clock setting at booting time.
Simply, clearing host->using_dma flag on dw_mci_dmac_complete_dma() and
dw_mci_stop_dma() like following fixes the issue, but I am not sure
this approach is proper.
---
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..a71c94b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -388,6 +388,7 @@ static void dw_mci_stop_dma(struct dw_mci *host)
if (host->using_dma) {
host->dma_ops->stop(host);
host->dma_ops->cleanup(host);
+ host->using_dma = 0;
}
/* Data transfer was stopped by the interrupt handler */
@@ -455,6 +456,7 @@ static void dw_mci_dmac_complete_dma(void *arg)
DMA_FROM_DEVICE);
host->dma_ops->cleanup(host);
+ host->using_dma = 0;
/*
* If the card was removed, data will be NULL. No point in trying to
@@ -943,8 +945,6 @@ static int dw_mci_submit_data_dma(struct dw_mci *host,
struct mmc_data *data)
int sg_len;
u32 temp;
- host->using_dma = 0;
-
/* If we don't have a channel, we can't do DMA */
if (!host->use_dma)
return -ENODEV;
---
Best Regards,
- Seung-Woo Kim
mmc: dw_mmc: warning with CONFIG_DMA_API_DEBUG
Hi folks,
During booting test on my Exynos5422 based Odroid-XU3, kernel compiled
with CONFIG_DMA_API_DEBUG reported following warning:
[ cut here ]
WARNING: CPU: 0 PID: 0 at lib/dma-debug.c:1096 check_unmap+0x7bc/0xb38
dwmmc_exynos 1220.mmc: DMA-API: device driver tries to free DMA memory it
has not allocated [device address=0x6d9d2200]
[size=128 bytes]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.7.0-rc4 #26
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[] (unwind_backtrace) from [] (show_stack+0x20/0x24)
[] (show_stack) from [] (dump_stack+0x80/0x94)
[] (dump_stack) from [] (__warn+0xf8/0x110)
[] (__warn) from [] (warn_slowpath_fmt+0x48/0x50)
[] (warn_slowpath_fmt) from [] (check_unmap+0x7bc/0xb38)
[] (check_unmap) from [] (debug_dma_unmap_sg+0x118/0x148)
[] (debug_dma_unmap_sg) from []
(dw_mci_dma_cleanup+0x7c/0xb8)
[] (dw_mci_dma_cleanup) from [] (dw_mci_stop_dma+0x40/0x50)
[] (dw_mci_stop_dma) from []
(dw_mci_tasklet_func+0x130/0x3b4)
[] (dw_mci_tasklet_func) from [] (tasklet_action+0xb4/0x150)
[] (tasklet_action) from [] (__do_softirq+0xe4/0x3cc)
[] (__do_softirq) from [] (irq_exit+0xd0/0x10c)
[] (irq_exit) from [] (__handle_domain_irq+0x90/0xfc)
[] (__handle_domain_irq) from [] (gic_handle_irq+0x64/0xa8)
[] (gic_handle_irq) from [] (__irq_svc+0x54/0x90)
Exception stack(0xc1101ef8 to 0xc1101f40)
1ee0: 0001
1f00: c011b600 c110 c110753c c11c3984 c11074d4 c1107548
1f20: c1101f54 c1101f58 c1101f48 c010a1fc c010a200 6013
[] (__irq_svc) from [] (arch_cpu_idle+0x48/0x4c)
[] (arch_cpu_idle) from [] (default_idle_call+0x30/0x3c)
[] (default_idle_call) from []
(cpu_startup_entry+0x358/0x3b4)
[] (cpu_startup_entry) from [] (rest_init+0x94/0x98)
[] (rest_init) from [] (start_kernel+0x3a4/0x3b0)
[] (start_kernel) from [<4000807c>] (0x4000807c)
---[ end trace 256f83eed365daf0 ]---
The warning occurs because after complete callback function,
dw_mci_dmac_complete_dma() is called, then dw_mci_stop_dma() is called
again. So it causes dma_unmap_sg() is called twice for same sg. It
occurs during clock setting at booting time.
Simply, clearing host->using_dma flag on dw_mci_dmac_complete_dma() and
dw_mci_stop_dma() like following fixes the issue, but I am not sure
this approach is proper.
---
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..a71c94b 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -388,6 +388,7 @@ static void dw_mci_stop_dma(struct dw_mci *host)
if (host->using_dma) {
host->dma_ops->stop(host);
host->dma_ops->cleanup(host);
+ host->using_dma = 0;
}
/* Data transfer was stopped by the interrupt handler */
@@ -455,6 +456,7 @@ static void dw_mci_dmac_complete_dma(void *arg)
DMA_FROM_DEVICE);
host->dma_ops->cleanup(host);
+ host->using_dma = 0;
/*
* If the card was removed, data will be NULL. No point in trying to
@@ -943,8 +945,6 @@ static int dw_mci_submit_data_dma(struct dw_mci *host,
struct mmc_data *data)
int sg_len;
u32 temp;
- host->using_dma = 0;
-
/* If we don't have a channel, we can't do DMA */
if (!host->use_dma)
return -ENODEV;
---
Best Regards,
- Seung-Woo Kim
[PATCH v3] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus().
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x3a0/0x438
[...]
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x384/0x438
[...]
The warnings are caused because of bit shift which is used to
filter spamming message for CONFIG_MMC_CLKGATE, but the config is
already removed. So this patch just removes the shift.
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
drivers/mmc/host/dw_mmc.c | 14 +-
drivers/mmc/host/dw_mmc.h |4
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..bada11e 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1099,12 +1099,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
- if ((clock << div) != slot->__clk_old || force_clkinit)
- dev_info(>mmc->class_dev,
-"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
-slot->id, host->bus_hz, clock,
-div ? ((host->bus_hz / div) >> 1) :
-host->bus_hz, div);
+ dev_info(>mmc->class_dev,
+"Bus speed (slot %d) = %dHz (slot req %dHz, actual
%dHZ div = %d)\n",
+slot->id, host->bus_hz, clock,
+div ? ((host->bus_hz / div) >> 1) :
+host->bus_hz, div);
/* disable clock */
mci_writel(host, CLKENA, 0);
@@ -1127,9 +1126,6 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
/* inform CIU */
mci_send_cmd(slot, sdmmc_cmd_bits, 0);
-
- /* keep the clock with reflecting clock dividor */
- slot->__clk_old = clock << div;
}
host->current_speed = clock;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 1e8d838..5961037 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -245,9 +245,6 @@ extern int dw_mci_resume(struct dw_mci *host);
* @queue_node: List node for placing this node in the @queue list of
* dw_mci.
* @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- * Keeping track of this helps us to avoid spamming the console
- * with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
* @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -262,7 +259,6 @@ struct dw_mci_slot {
struct list_headqueue_node;
unsigned intclock;
- unsigned int__clk_old;
unsigned long flags;
#define DW_MMC_CARD_PRESENT0
--
1.7.9.5
[PATCH v3] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus().
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x3a0/0x438
[...]
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x384/0x438
[...]
The warnings are caused because of bit shift which is used to
filter spamming message for CONFIG_MMC_CLKGATE, but the config is
already removed. So this patch just removes the shift.
Signed-off-by: Seung-Woo Kim
---
drivers/mmc/host/dw_mmc.c | 14 +-
drivers/mmc/host/dw_mmc.h |4
2 files changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..bada11e 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1099,12 +1099,11 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
- if ((clock << div) != slot->__clk_old || force_clkinit)
- dev_info(>mmc->class_dev,
-"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
-slot->id, host->bus_hz, clock,
-div ? ((host->bus_hz / div) >> 1) :
-host->bus_hz, div);
+ dev_info(>mmc->class_dev,
+"Bus speed (slot %d) = %dHz (slot req %dHz, actual
%dHZ div = %d)\n",
+slot->id, host->bus_hz, clock,
+div ? ((host->bus_hz / div) >> 1) :
+host->bus_hz, div);
/* disable clock */
mci_writel(host, CLKENA, 0);
@@ -1127,9 +1126,6 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
/* inform CIU */
mci_send_cmd(slot, sdmmc_cmd_bits, 0);
-
- /* keep the clock with reflecting clock dividor */
- slot->__clk_old = clock << div;
}
host->current_speed = clock;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 1e8d838..5961037 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -245,9 +245,6 @@ extern int dw_mci_resume(struct dw_mci *host);
* @queue_node: List node for placing this node in the @queue list of
* dw_mci.
* @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- * Keeping track of this helps us to avoid spamming the console
- * with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
* @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -262,7 +259,6 @@ struct dw_mci_slot {
struct list_headqueue_node;
unsigned intclock;
- unsigned int__clk_old;
unsigned long flags;
#define DW_MMC_CARD_PRESENT0
--
1.7.9.5
RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hi, > -Original Message- > From: Seung-Woo Kim [mailto:sw0312@samsung.com] > Sent: Monday, June 20, 2016 12:30 PM > To: 'Jaehoon Chung'; ulf.hans...@linaro.org; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org > Cc: sw0312@samsung.com > Subject: RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in > dw_mci_setup_bus() > > Hello Jaehoon, > > > -Original Message- > > From: Jaehoon Chung [mailto:jh80.ch...@samsung.com] > > Sent: Monday, June 20, 2016 11:34 AM > > To: Seung-Woo Kim; ulf.hans...@linaro.org; linux-...@vger.kernel.org; > > linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in > > dw_mci_setup_bus() > > > > Hi SeungWoo, > > > > On 06/17/2016 02:16 PM, Seung-Woo Kim wrote: > > > This patch removes following UBSAN warnings in dw_mci_setup_bus(). > > > > > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 > > > shift exponent 250 is too large for 32-bit type 'unsigned int' > > > Call trace: > > > [] dump_backtrace+0x0/0x380 > > > [] show_stack+0x14/0x20 > > > [] dump_stack+0xe0/0x120 > > > [] ubsan_epilogue+0x18/0x68 > > > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > > > [] dw_mci_setup_bus+0x3a0/0x438 > > > [...] > > > > > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 > > > shift exponent 250 is too large for 32-bit type 'unsigned int' > > > Call trace: > > > [] dump_backtrace+0x0/0x380 > > > [] show_stack+0x14/0x20 > > > [] dump_stack+0xe0/0x120 > > > [] ubsan_epilogue+0x18/0x68 > > > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > > > [] dw_mci_setup_bus+0x384/0x438 > > > [...] > > > > > > The warnings are caused because of shift with more than 31 on 32 > > > bit variable, so this patch fixes to keep both clock and divider > > > instead of shift. > > > > > > Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> > > > --- > > > drivers/mmc/host/dw_mmc.c |8 +--- > > > drivers/mmc/host/dw_mmc.h |8 +--- > > > 2 files changed, 10 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > > index 2cc6123..d05c8cc 100644 > > > --- a/drivers/mmc/host/dw_mmc.c > > > +++ b/drivers/mmc/host/dw_mmc.c > > > @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot > > > *slot, bool force_clkinit) > > > > > > div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; > > > > > > - if ((clock << div) != slot->__clk_old || force_clkinit) > > > + if (clock != slot->__clk_old || div != slot->__div_old || > > > + force_clkinit) > > > > I have realized why we put this condition..after checking your below codes. > > This was patch to avoid the spamming for CONFIG_MMC_CLKGATE.. > > but CONFIG_MMC_CLKAGET didn't use anymore.. I think we can remove this > > condition. > > > > How about? > > There is no CONFIG_MMC_CLKAGET, but message is still printed. So without > the condition check, there will be too much log. If you will just remove > below print, then I agree about removing the condition and __clk_old, instead > of my patch. I tested again after removing the condition check, it prints same log and there is not too much log. Let's just remove the condition and __clk_old. Regards, - Seung-Woo Kim > > Thanks, > - Seung-Woo Kim > > > > > Best Regards, > > Jaehoon Chung > > > > > dev_info(>mmc->class_dev, > > >"Bus speed (slot %d) = %dHz (slot req %dHz, > > > actual %dHZ div = %d)\n", > > >slot->id, host->bus_hz, clock, > > > @@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot > > > *slot, bool force_clkinit) > > > /* inform CIU */ > > > mci_send_cmd(slot, sdmmc_cmd_bits, 0); > > > > > > - /* keep the clock with reflecting clock dividor */ > > > - slot->__clk_old = clock << div; > > > + /* keep the clock and clock divider */ > > > + slot->__clk_old = clock; > > > + slot->__div_old = div; > > > } > > > > > > host->current_speed = clock; > > > diff --git a/d
RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hi, > -Original Message- > From: Seung-Woo Kim [mailto:sw0312@samsung.com] > Sent: Monday, June 20, 2016 12:30 PM > To: 'Jaehoon Chung'; ulf.hans...@linaro.org; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org > Cc: sw0312@samsung.com > Subject: RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in > dw_mci_setup_bus() > > Hello Jaehoon, > > > -Original Message- > > From: Jaehoon Chung [mailto:jh80.ch...@samsung.com] > > Sent: Monday, June 20, 2016 11:34 AM > > To: Seung-Woo Kim; ulf.hans...@linaro.org; linux-...@vger.kernel.org; > > linux-kernel@vger.kernel.org > > Subject: Re: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in > > dw_mci_setup_bus() > > > > Hi SeungWoo, > > > > On 06/17/2016 02:16 PM, Seung-Woo Kim wrote: > > > This patch removes following UBSAN warnings in dw_mci_setup_bus(). > > > > > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 > > > shift exponent 250 is too large for 32-bit type 'unsigned int' > > > Call trace: > > > [] dump_backtrace+0x0/0x380 > > > [] show_stack+0x14/0x20 > > > [] dump_stack+0xe0/0x120 > > > [] ubsan_epilogue+0x18/0x68 > > > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > > > [] dw_mci_setup_bus+0x3a0/0x438 > > > [...] > > > > > > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 > > > shift exponent 250 is too large for 32-bit type 'unsigned int' > > > Call trace: > > > [] dump_backtrace+0x0/0x380 > > > [] show_stack+0x14/0x20 > > > [] dump_stack+0xe0/0x120 > > > [] ubsan_epilogue+0x18/0x68 > > > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc > > > [] dw_mci_setup_bus+0x384/0x438 > > > [...] > > > > > > The warnings are caused because of shift with more than 31 on 32 > > > bit variable, so this patch fixes to keep both clock and divider > > > instead of shift. > > > > > > Signed-off-by: Seung-Woo Kim > > > --- > > > drivers/mmc/host/dw_mmc.c |8 +--- > > > drivers/mmc/host/dw_mmc.h |8 +--- > > > 2 files changed, 10 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > > > index 2cc6123..d05c8cc 100644 > > > --- a/drivers/mmc/host/dw_mmc.c > > > +++ b/drivers/mmc/host/dw_mmc.c > > > @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot > > > *slot, bool force_clkinit) > > > > > > div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; > > > > > > - if ((clock << div) != slot->__clk_old || force_clkinit) > > > + if (clock != slot->__clk_old || div != slot->__div_old || > > > + force_clkinit) > > > > I have realized why we put this condition..after checking your below codes. > > This was patch to avoid the spamming for CONFIG_MMC_CLKGATE.. > > but CONFIG_MMC_CLKAGET didn't use anymore.. I think we can remove this > > condition. > > > > How about? > > There is no CONFIG_MMC_CLKAGET, but message is still printed. So without > the condition check, there will be too much log. If you will just remove > below print, then I agree about removing the condition and __clk_old, instead > of my patch. I tested again after removing the condition check, it prints same log and there is not too much log. Let's just remove the condition and __clk_old. Regards, - Seung-Woo Kim > > Thanks, > - Seung-Woo Kim > > > > > Best Regards, > > Jaehoon Chung > > > > > dev_info(>mmc->class_dev, > > >"Bus speed (slot %d) = %dHz (slot req %dHz, > > > actual %dHZ div = %d)\n", > > >slot->id, host->bus_hz, clock, > > > @@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot > > > *slot, bool force_clkinit) > > > /* inform CIU */ > > > mci_send_cmd(slot, sdmmc_cmd_bits, 0); > > > > > > - /* keep the clock with reflecting clock dividor */ > > > - slot->__clk_old = clock << div; > > > + /* keep the clock and clock divider */ > > > + slot->__clk_old = clock; > > > + slot->__div_old = div; > > > } > > > > > > host->current_speed = clock; > > > diff --git a/drivers/mmc/host/dw_mmc.h b/dri
RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hello Jaehoon,
> -Original Message-
> From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
> Sent: Monday, June 20, 2016 11:34 AM
> To: Seung-Woo Kim; ulf.hans...@linaro.org; linux-...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in
> dw_mci_setup_bus()
>
> Hi SeungWoo,
>
> On 06/17/2016 02:16 PM, Seung-Woo Kim wrote:
> > This patch removes following UBSAN warnings in dw_mci_setup_bus().
> >
> > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
> > shift exponent 250 is too large for 32-bit type 'unsigned int'
> > Call trace:
> > [] dump_backtrace+0x0/0x380
> > [] show_stack+0x14/0x20
> > [] dump_stack+0xe0/0x120
> > [] ubsan_epilogue+0x18/0x68
> > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
> > [] dw_mci_setup_bus+0x3a0/0x438
> > [...]
> >
> > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
> > shift exponent 250 is too large for 32-bit type 'unsigned int'
> > Call trace:
> > [] dump_backtrace+0x0/0x380
> > [] show_stack+0x14/0x20
> > [] dump_stack+0xe0/0x120
> > [] ubsan_epilogue+0x18/0x68
> > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
> > [] dw_mci_setup_bus+0x384/0x438
> > [...]
> >
> > The warnings are caused because of shift with more than 31 on 32
> > bit variable, so this patch fixes to keep both clock and divider
> > instead of shift.
> >
> > Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
> > ---
> > drivers/mmc/host/dw_mmc.c |8 +---
> > drivers/mmc/host/dw_mmc.h |8 +---
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 2cc6123..d05c8cc 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot
> > *slot, bool force_clkinit)
> >
> > div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
> >
> > - if ((clock << div) != slot->__clk_old || force_clkinit)
> > + if (clock != slot->__clk_old || div != slot->__div_old ||
> > + force_clkinit)
>
> I have realized why we put this condition..after checking your below codes.
> This was patch to avoid the spamming for CONFIG_MMC_CLKGATE..
> but CONFIG_MMC_CLKAGET didn't use anymore.. I think we can remove this
> condition.
>
> How about?
There is no CONFIG_MMC_CLKAGET, but message is still printed. So without
the condition check, there will be too much log. If you will just remove
below print, then I agree about removing the condition and __clk_old, instead
of my patch.
Thanks,
- Seung-Woo Kim
>
> Best Regards,
> Jaehoon Chung
>
> > dev_info(>mmc->class_dev,
> > "Bus speed (slot %d) = %dHz (slot req %dHz,
> > actual %dHZ div = %d)\n",
> > slot->id, host->bus_hz, clock,
> > @@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot
> > *slot, bool force_clkinit)
> > /* inform CIU */
> > mci_send_cmd(slot, sdmmc_cmd_bits, 0);
> >
> > - /* keep the clock with reflecting clock dividor */
> > - slot->__clk_old = clock << div;
> > + /* keep the clock and clock divider */
> > + slot->__clk_old = clock;
> > + slot->__div_old = div;
> > }
> >
> > host->current_speed = clock;
> > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> > index 1e8d838..fdfc3f5 100644
> > --- a/drivers/mmc/host/dw_mmc.h
> > +++ b/drivers/mmc/host/dw_mmc.h
> > @@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host);
> > * @queue_node: List node for placing this node in the @queue list of
> > * dw_mci.
> > * @clock: Clock rate configured by set_ios(). Protected by host->lock.
> > - * @__clk_old: The last updated clock with reflecting clock divider.
> > - * Keeping track of this helps us to avoid spamming the console
> > - * with CONFIG_MMC_CLKGATE.
> > + * @__clk_old: The last updated clock.
> > + * @__div_old: The last updated clock divider.
> > + * Keeping track of clock and clock divider helps us to avoid spamming
> > + * the console with CONFIG_MMC_CLKGATE.
> > * @flags: Random state bits associated with the slot.
> > * @id: Number of this slot.
> > * @sdio_id: Number of this slot in the SDIO interrupt registers.
> > @@ -263,6 +264,7 @@ struct dw_mci_slot {
> >
> > unsigned intclock;
> > unsigned int__clk_old;
> > + unsigned int__div_old;
> >
> > unsigned long flags;
> > #define DW_MMC_CARD_PRESENT0
> >
RE: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hello Jaehoon,
> -Original Message-
> From: Jaehoon Chung [mailto:jh80.ch...@samsung.com]
> Sent: Monday, June 20, 2016 11:34 AM
> To: Seung-Woo Kim; ulf.hans...@linaro.org; linux-...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v2] mmc: dw_mmc: remove UBSAN warning in
> dw_mci_setup_bus()
>
> Hi SeungWoo,
>
> On 06/17/2016 02:16 PM, Seung-Woo Kim wrote:
> > This patch removes following UBSAN warnings in dw_mci_setup_bus().
> >
> > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
> > shift exponent 250 is too large for 32-bit type 'unsigned int'
> > Call trace:
> > [] dump_backtrace+0x0/0x380
> > [] show_stack+0x14/0x20
> > [] dump_stack+0xe0/0x120
> > [] ubsan_epilogue+0x18/0x68
> > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
> > [] dw_mci_setup_bus+0x3a0/0x438
> > [...]
> >
> > UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
> > shift exponent 250 is too large for 32-bit type 'unsigned int'
> > Call trace:
> > [] dump_backtrace+0x0/0x380
> > [] show_stack+0x14/0x20
> > [] dump_stack+0xe0/0x120
> > [] ubsan_epilogue+0x18/0x68
> > [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
> > [] dw_mci_setup_bus+0x384/0x438
> > [...]
> >
> > The warnings are caused because of shift with more than 31 on 32
> > bit variable, so this patch fixes to keep both clock and divider
> > instead of shift.
> >
> > Signed-off-by: Seung-Woo Kim
> > ---
> > drivers/mmc/host/dw_mmc.c |8 +---
> > drivers/mmc/host/dw_mmc.h |8 +---
> > 2 files changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
> > index 2cc6123..d05c8cc 100644
> > --- a/drivers/mmc/host/dw_mmc.c
> > +++ b/drivers/mmc/host/dw_mmc.c
> > @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot
> > *slot, bool force_clkinit)
> >
> > div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
> >
> > - if ((clock << div) != slot->__clk_old || force_clkinit)
> > + if (clock != slot->__clk_old || div != slot->__div_old ||
> > + force_clkinit)
>
> I have realized why we put this condition..after checking your below codes.
> This was patch to avoid the spamming for CONFIG_MMC_CLKGATE..
> but CONFIG_MMC_CLKAGET didn't use anymore.. I think we can remove this
> condition.
>
> How about?
There is no CONFIG_MMC_CLKAGET, but message is still printed. So without
the condition check, there will be too much log. If you will just remove
below print, then I agree about removing the condition and __clk_old, instead
of my patch.
Thanks,
- Seung-Woo Kim
>
> Best Regards,
> Jaehoon Chung
>
> > dev_info(>mmc->class_dev,
> > "Bus speed (slot %d) = %dHz (slot req %dHz,
> > actual %dHZ div = %d)\n",
> > slot->id, host->bus_hz, clock,
> > @@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot
> > *slot, bool force_clkinit)
> > /* inform CIU */
> > mci_send_cmd(slot, sdmmc_cmd_bits, 0);
> >
> > - /* keep the clock with reflecting clock dividor */
> > - slot->__clk_old = clock << div;
> > + /* keep the clock and clock divider */
> > + slot->__clk_old = clock;
> > + slot->__div_old = div;
> > }
> >
> > host->current_speed = clock;
> > diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
> > index 1e8d838..fdfc3f5 100644
> > --- a/drivers/mmc/host/dw_mmc.h
> > +++ b/drivers/mmc/host/dw_mmc.h
> > @@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host);
> > * @queue_node: List node for placing this node in the @queue list of
> > * dw_mci.
> > * @clock: Clock rate configured by set_ios(). Protected by host->lock.
> > - * @__clk_old: The last updated clock with reflecting clock divider.
> > - * Keeping track of this helps us to avoid spamming the console
> > - * with CONFIG_MMC_CLKGATE.
> > + * @__clk_old: The last updated clock.
> > + * @__div_old: The last updated clock divider.
> > + * Keeping track of clock and clock divider helps us to avoid spamming
> > + * the console with CONFIG_MMC_CLKGATE.
> > * @flags: Random state bits associated with the slot.
> > * @id: Number of this slot.
> > * @sdio_id: Number of this slot in the SDIO interrupt registers.
> > @@ -263,6 +264,7 @@ struct dw_mci_slot {
> >
> > unsigned intclock;
> > unsigned int__clk_old;
> > + unsigned int__div_old;
> >
> > unsigned long flags;
> > #define DW_MMC_CARD_PRESENT0
> >
[PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus().
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x3a0/0x438
[...]
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x384/0x438
[...]
The warnings are caused because of shift with more than 31 on 32
bit variable, so this patch fixes to keep both clock and divider
instead of shift.
Signed-off-by: Seung-Woo Kim <sw0312@samsung.com>
---
drivers/mmc/host/dw_mmc.c |8 +---
drivers/mmc/host/dw_mmc.h |8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..d05c8cc 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
- if ((clock << div) != slot->__clk_old || force_clkinit)
+ if (clock != slot->__clk_old || div != slot->__div_old ||
+ force_clkinit)
dev_info(>mmc->class_dev,
"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
slot->id, host->bus_hz, clock,
@@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
/* inform CIU */
mci_send_cmd(slot, sdmmc_cmd_bits, 0);
- /* keep the clock with reflecting clock dividor */
- slot->__clk_old = clock << div;
+ /* keep the clock and clock divider */
+ slot->__clk_old = clock;
+ slot->__div_old = div;
}
host->current_speed = clock;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 1e8d838..fdfc3f5 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host);
* @queue_node: List node for placing this node in the @queue list of
* dw_mci.
* @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- * Keeping track of this helps us to avoid spamming the console
- * with CONFIG_MMC_CLKGATE.
+ * @__clk_old: The last updated clock.
+ * @__div_old: The last updated clock divider.
+ * Keeping track of clock and clock divider helps us to avoid spamming
+ * the console with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
* @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -263,6 +264,7 @@ struct dw_mci_slot {
unsigned intclock;
unsigned int__clk_old;
+ unsigned int__div_old;
unsigned long flags;
#define DW_MMC_CARD_PRESENT0
--
1.7.4.1
[PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus().
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x3a0/0x438
[...]
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x384/0x438
[...]
The warnings are caused because of shift with more than 31 on 32
bit variable, so this patch fixes to keep both clock and divider
instead of shift.
Signed-off-by: Seung-Woo Kim
---
drivers/mmc/host/dw_mmc.c |8 +---
drivers/mmc/host/dw_mmc.h |8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..d05c8cc 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
- if ((clock << div) != slot->__clk_old || force_clkinit)
+ if (clock != slot->__clk_old || div != slot->__div_old ||
+ force_clkinit)
dev_info(>mmc->class_dev,
"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
slot->id, host->bus_hz, clock,
@@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
/* inform CIU */
mci_send_cmd(slot, sdmmc_cmd_bits, 0);
- /* keep the clock with reflecting clock dividor */
- slot->__clk_old = clock << div;
+ /* keep the clock and clock divider */
+ slot->__clk_old = clock;
+ slot->__div_old = div;
}
host->current_speed = clock;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 1e8d838..fdfc3f5 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host);
* @queue_node: List node for placing this node in the @queue list of
* dw_mci.
* @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- * Keeping track of this helps us to avoid spamming the console
- * with CONFIG_MMC_CLKGATE.
+ * @__clk_old: The last updated clock.
+ * @__div_old: The last updated clock divider.
+ * Keeping track of clock and clock divider helps us to avoid spamming
+ * the console with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
* @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -263,6 +264,7 @@ struct dw_mci_slot {
unsigned intclock;
unsigned int__clk_old;
+ unsigned int__div_old;
unsigned long flags;
#define DW_MMC_CARD_PRESENT0
--
1.7.4.1
Re: [PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hello Jaehoon, On 2016년 06월 17일 10:30, Jaehoon Chung wrote: > Hi Seung-Woo, > > On 06/10/2016 10:29 AM, Seung-Woo Kim wrote: >> Hi Jaehoon, >> >> On 2016년 06월 09일 21:38, Jaehoon Chung wrote: >>> Hi Seung-Woo, >>> >>> On 06/08/2016 01:07 PM, Seung-Woo Kim wrote: >>>> This patch removes following UBSAN warnings in dw_mci_setup_bus(). >>>> The warnings are caused because of shift with more than 31 on 32 >>>> bit variable, so this patch fixes to shift only for less than 32. >>>> >>>> >>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>>> index 2cc6123..dff045e 100644 >>>> --- a/drivers/mmc/host/dw_mmc.c >>>> +++ b/drivers/mmc/host/dw_mmc.c >>>> @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot >>>> *slot, bool force_clkinit) >>>> >>>>div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; >>>> >>>> - if ((clock << div) != slot->__clk_old || force_clkinit) >>>> + if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || >>> >>> Well, we don't expect that clock is 0. >>> if clock is 0, it should be passed to " if (!clock)".. >>> >>> During initializing card, clock is 400KHz or less..I understood what you >>> want to fix. >>> But I taught this is not correct. >> >> It seems slot->__clk_old is not really clock but a kind of value to >> store both clock and div in one variable. And at least, my compiler >> calculates right shift with more than 32 as 0. I am not sure there is >> other proper value for the case. >> >> By the way, in my test environment, clock is calculated as like >> following steps: >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 40Hz, >> actual 40HZ div = 250) >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 2Hz, >> actual 2HZ div = 0) >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 5200Hz, >> actual 5000HZ div = 2) >> mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 5200Hz, >> actual 5000HZ div = 4) >> mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 2Hz, >> actual 2HZ div = 1) >> >> and only the first case is reported as the warning. If you let me know >> any proper value, then I will fix with the value. > > I think that "clock << div" and__old_clock are used for checking whether > clock is changed. > So it doesn't need to rotate with div. > > Could you check the below codes? > > if ((clock != slot->clk_old) || force_clkinit) > ... > > slot->__clk_old = clock; >From the comment about alignment of slot->__clk_old, it says that "keep the clock with reflecting clock dividor". So to me, it is better also adding slot->__dlv with your suggestion. I will send patch like it, soon. Regards, - Seung-Woo Kim > > Best Regards, > Jaehoon Chung > >> >> Thanks, >> - Seung-Woo Kim >> >> >>> >>> >>>> + force_clkinit) >>>>dev_info(>mmc->class_dev, >>>> "Bus speed (slot %d) = %dHz (slot req %dHz, >>>> actual %dHZ div = %d)\n", >>>> slot->id, host->bus_hz, clock, >>>> @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot >>>> *slot, bool force_clkinit) >>>>mci_send_cmd(slot, sdmmc_cmd_bits, 0); >>>> >>>>/* keep the clock with reflecting clock dividor */ >>>> - slot->__clk_old = clock << div; >>>> + slot->__clk_old = (div < 32) ? (clock << div) : 0; -- Seung-Woo Kim Samsung Software R Center --
Re: [PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hello Jaehoon, On 2016년 06월 17일 10:30, Jaehoon Chung wrote: > Hi Seung-Woo, > > On 06/10/2016 10:29 AM, Seung-Woo Kim wrote: >> Hi Jaehoon, >> >> On 2016년 06월 09일 21:38, Jaehoon Chung wrote: >>> Hi Seung-Woo, >>> >>> On 06/08/2016 01:07 PM, Seung-Woo Kim wrote: >>>> This patch removes following UBSAN warnings in dw_mci_setup_bus(). >>>> The warnings are caused because of shift with more than 31 on 32 >>>> bit variable, so this patch fixes to shift only for less than 32. >>>> >>>> >>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >>>> index 2cc6123..dff045e 100644 >>>> --- a/drivers/mmc/host/dw_mmc.c >>>> +++ b/drivers/mmc/host/dw_mmc.c >>>> @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot >>>> *slot, bool force_clkinit) >>>> >>>>div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; >>>> >>>> - if ((clock << div) != slot->__clk_old || force_clkinit) >>>> + if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || >>> >>> Well, we don't expect that clock is 0. >>> if clock is 0, it should be passed to " if (!clock)".. >>> >>> During initializing card, clock is 400KHz or less..I understood what you >>> want to fix. >>> But I taught this is not correct. >> >> It seems slot->__clk_old is not really clock but a kind of value to >> store both clock and div in one variable. And at least, my compiler >> calculates right shift with more than 32 as 0. I am not sure there is >> other proper value for the case. >> >> By the way, in my test environment, clock is calculated as like >> following steps: >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 40Hz, >> actual 40HZ div = 250) >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 2Hz, >> actual 2HZ div = 0) >> mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 5200Hz, >> actual 5000HZ div = 2) >> mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 5200Hz, >> actual 5000HZ div = 4) >> mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 2Hz, >> actual 2HZ div = 1) >> >> and only the first case is reported as the warning. If you let me know >> any proper value, then I will fix with the value. > > I think that "clock << div" and__old_clock are used for checking whether > clock is changed. > So it doesn't need to rotate with div. > > Could you check the below codes? > > if ((clock != slot->clk_old) || force_clkinit) > ... > > slot->__clk_old = clock; >From the comment about alignment of slot->__clk_old, it says that "keep the clock with reflecting clock dividor". So to me, it is better also adding slot->__dlv with your suggestion. I will send patch like it, soon. Regards, - Seung-Woo Kim > > Best Regards, > Jaehoon Chung > >> >> Thanks, >> - Seung-Woo Kim >> >> >>> >>> >>>> + force_clkinit) >>>>dev_info(>mmc->class_dev, >>>> "Bus speed (slot %d) = %dHz (slot req %dHz, >>>> actual %dHZ div = %d)\n", >>>> slot->id, host->bus_hz, clock, >>>> @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot >>>> *slot, bool force_clkinit) >>>>mci_send_cmd(slot, sdmmc_cmd_bits, 0); >>>> >>>>/* keep the clock with reflecting clock dividor */ >>>> - slot->__clk_old = clock << div; >>>> + slot->__clk_old = (div < 32) ? (clock << div) : 0; -- Seung-Woo Kim Samsung Software R Center --
Re: [PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hi Jaehoon, On 2016년 06월 09일 21:38, Jaehoon Chung wrote: > Hi Seung-Woo, > > On 06/08/2016 01:07 PM, Seung-Woo Kim wrote: >> This patch removes following UBSAN warnings in dw_mci_setup_bus(). >> The warnings are caused because of shift with more than 31 on 32 >> bit variable, so this patch fixes to shift only for less than 32. >> >> UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 >> shift exponent 250 is too large for 32-bit type 'unsigned int' >> Call trace: >> [] dump_backtrace+0x0/0x380 >> [] show_stack+0x14/0x20 >> [] dump_stack+0xe0/0x120 >> [] ubsan_epilogue+0x18/0x68 >> [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc >> [] dw_mci_setup_bus+0x3a0/0x438 >> [...] >> >> UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 >> shift exponent 250 is too large for 32-bit type 'unsigned int' >> Call trace: >> [] dump_backtrace+0x0/0x380 >> [] show_stack+0x14/0x20 >> [] dump_stack+0xe0/0x120 >> [] ubsan_epilogue+0x18/0x68 >> [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc >> [] dw_mci_setup_bus+0x384/0x438 > > The below message can be discarded. You are right, like above message, below part can be replaced as [...]. > >> [] dw_mci_set_ios+0x184/0x798 >> [] mmc_power_up+0x11c/0x260 >> [] mmc_start_host+0x88/0x100 >> [] mmc_add_host+0x6c/0x128 >> [] dw_mci_probe+0x1088/0x1750 >> [] dw_mci_pltfm_register+0x108/0x178 >> [] dw_mci_exynos_probe+0x4c/0x88 >> [] platform_drv_probe+0x78/0x180 >> [] driver_probe_device+0x144/0x460 >> [] __driver_attach+0xf4/0x140 >> [] bus_for_each_dev+0xf0/0x160 >> [] driver_attach+0x34/0x58 >> [] bus_add_driver+0x2c0/0x398 >> [] driver_register+0xbc/0x1e0 >> [] __platform_driver_register+0x84/0xa8 >> [] dw_mci_exynos_pltfm_driver_init+0x18/0x20 >> [] do_one_initcall+0xa0/0x2c8 >> [] kernel_init_freeable+0x52c/0x5dc >> [] kernel_init+0x1c/0xf8 >> [] ret_from_fork+0x10/0x40 >> >> Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> >> --- >> drivers/mmc/host/dw_mmc.c |5 +++-- >> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 2cc6123..dff045e 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, >> bool force_clkinit) >> >> div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; >> >> -if ((clock << div) != slot->__clk_old || force_clkinit) >> +if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || > > Well, we don't expect that clock is 0. > if clock is 0, it should be passed to " if (!clock)".. > > During initializing card, clock is 400KHz or less..I understood what you want > to fix. > But I taught this is not correct. It seems slot->__clk_old is not really clock but a kind of value to store both clock and div in one variable. And at least, my compiler calculates right shift with more than 32 as 0. I am not sure there is other proper value for the case. By the way, in my test environment, clock is calculated as like following steps: mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 40Hz, actual 40HZ div = 250) mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 2Hz, actual 200000000HZ div = 0) mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 5200Hz, actual 5000HZ div = 2) mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 5200Hz, actual 5000HZ div = 4) mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 2Hz, actual 2HZ div = 1) and only the first case is reported as the warning. If you let me know any proper value, then I will fix with the value. Thanks, - Seung-Woo Kim > > >> +force_clkinit) >> dev_info(>mmc->class_dev, >> "Bus speed (slot %d) = %dHz (slot req %dHz, >> actual %dHZ div = %d)\n", >> slot->id, host->bus_hz, clock, >> @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, >> bool force_clkinit) >> mci_send_cmd(slot, sdmmc_cmd_bits, 0); >> >> /* keep the clock with reflecting clock dividor */ >> -slot->__clk_old = clock << div; >> +slot->__clk_old = (div < 32) ? (clock << div) : 0; > > Also, clock is not 0.. > > > Best Regards, > Jaehoon Chung > >> } >> >> host->current_speed = clock; >> > > > -- Seung-Woo Kim Samsung Software R Center --
Re: [PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
Hi Jaehoon, On 2016년 06월 09일 21:38, Jaehoon Chung wrote: > Hi Seung-Woo, > > On 06/08/2016 01:07 PM, Seung-Woo Kim wrote: >> This patch removes following UBSAN warnings in dw_mci_setup_bus(). >> The warnings are caused because of shift with more than 31 on 32 >> bit variable, so this patch fixes to shift only for less than 32. >> >> UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 >> shift exponent 250 is too large for 32-bit type 'unsigned int' >> Call trace: >> [] dump_backtrace+0x0/0x380 >> [] show_stack+0x14/0x20 >> [] dump_stack+0xe0/0x120 >> [] ubsan_epilogue+0x18/0x68 >> [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc >> [] dw_mci_setup_bus+0x3a0/0x438 >> [...] >> >> UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 >> shift exponent 250 is too large for 32-bit type 'unsigned int' >> Call trace: >> [] dump_backtrace+0x0/0x380 >> [] show_stack+0x14/0x20 >> [] dump_stack+0xe0/0x120 >> [] ubsan_epilogue+0x18/0x68 >> [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc >> [] dw_mci_setup_bus+0x384/0x438 > > The below message can be discarded. You are right, like above message, below part can be replaced as [...]. > >> [] dw_mci_set_ios+0x184/0x798 >> [] mmc_power_up+0x11c/0x260 >> [] mmc_start_host+0x88/0x100 >> [] mmc_add_host+0x6c/0x128 >> [] dw_mci_probe+0x1088/0x1750 >> [] dw_mci_pltfm_register+0x108/0x178 >> [] dw_mci_exynos_probe+0x4c/0x88 >> [] platform_drv_probe+0x78/0x180 >> [] driver_probe_device+0x144/0x460 >> [] __driver_attach+0xf4/0x140 >> [] bus_for_each_dev+0xf0/0x160 >> [] driver_attach+0x34/0x58 >> [] bus_add_driver+0x2c0/0x398 >> [] driver_register+0xbc/0x1e0 >> [] __platform_driver_register+0x84/0xa8 >> [] dw_mci_exynos_pltfm_driver_init+0x18/0x20 >> [] do_one_initcall+0xa0/0x2c8 >> [] kernel_init_freeable+0x52c/0x5dc >> [] kernel_init+0x1c/0xf8 >> [] ret_from_fork+0x10/0x40 >> >> Signed-off-by: Seung-Woo Kim >> --- >> drivers/mmc/host/dw_mmc.c |5 +++-- >> 1 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c >> index 2cc6123..dff045e 100644 >> --- a/drivers/mmc/host/dw_mmc.c >> +++ b/drivers/mmc/host/dw_mmc.c >> @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, >> bool force_clkinit) >> >> div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; >> >> -if ((clock << div) != slot->__clk_old || force_clkinit) >> +if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || > > Well, we don't expect that clock is 0. > if clock is 0, it should be passed to " if (!clock)".. > > During initializing card, clock is 400KHz or less..I understood what you want > to fix. > But I taught this is not correct. It seems slot->__clk_old is not really clock but a kind of value to store both clock and div in one variable. And at least, my compiler calculates right shift with more than 32 as 0. I am not sure there is other proper value for the case. By the way, in my test environment, clock is calculated as like following steps: mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 40Hz, actual 40HZ div = 250) mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 2Hz, actual 2HZ div = 0) mmc_host mmc0: Bus speed (slot 0) = 2Hz (slot req 5200Hz, actual 5000HZ div = 2) mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 5200Hz, actual 5000HZ div = 4) mmc_host mmc0: Bus speed (slot 0) = 4Hz (slot req 2Hz, actual 2HZ div = 1) and only the first case is reported as the warning. If you let me know any proper value, then I will fix with the value. Thanks, - Seung-Woo Kim > > >> +force_clkinit) >> dev_info(>mmc->class_dev, >> "Bus speed (slot %d) = %dHz (slot req %dHz, >> actual %dHZ div = %d)\n", >> slot->id, host->bus_hz, clock, >> @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, >> bool force_clkinit) >> mci_send_cmd(slot, sdmmc_cmd_bits, 0); >> >> /* keep the clock with reflecting clock dividor */ >> -slot->__clk_old = clock << div; >> +slot->__clk_old = (div < 32) ? (clock << div) : 0; > > Also, clock is not 0.. > > > Best Regards, > Jaehoon Chung > >> } >> >> host->current_speed = clock; >> > > > -- Seung-Woo Kim Samsung Software R Center --
[PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus(). The warnings are caused because of shift with more than 31 on 32 bit variable, so this patch fixes to shift only for less than 32. UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 shift exponent 250 is too large for 32-bit type 'unsigned int' Call trace: [] dump_backtrace+0x0/0x380 [] show_stack+0x14/0x20 [] dump_stack+0xe0/0x120 [] ubsan_epilogue+0x18/0x68 [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc [] dw_mci_setup_bus+0x3a0/0x438 [...] UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 shift exponent 250 is too large for 32-bit type 'unsigned int' Call trace: [] dump_backtrace+0x0/0x380 [] show_stack+0x14/0x20 [] dump_stack+0xe0/0x120 [] ubsan_epilogue+0x18/0x68 [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc [] dw_mci_setup_bus+0x384/0x438 [] dw_mci_set_ios+0x184/0x798 [] mmc_power_up+0x11c/0x260 [] mmc_start_host+0x88/0x100 [] mmc_add_host+0x6c/0x128 [] dw_mci_probe+0x1088/0x1750 [] dw_mci_pltfm_register+0x108/0x178 [] dw_mci_exynos_probe+0x4c/0x88 [] platform_drv_probe+0x78/0x180 [] driver_probe_device+0x144/0x460 [] __driver_attach+0xf4/0x140 [] bus_for_each_dev+0xf0/0x160 [] driver_attach+0x34/0x58 [] bus_add_driver+0x2c0/0x398 [] driver_register+0xbc/0x1e0 [] __platform_driver_register+0x84/0xa8 [] dw_mci_exynos_pltfm_driver_init+0x18/0x20 [] do_one_initcall+0xa0/0x2c8 [] kernel_init_freeable+0x52c/0x5dc [] kernel_init+0x1c/0xf8 [] ret_from_fork+0x10/0x40 Signed-off-by: Seung-Woo Kim <sw0312@samsung.com> --- drivers/mmc/host/dw_mmc.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 2cc6123..dff045e 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; - if ((clock << div) != slot->__clk_old || force_clkinit) + if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || + force_clkinit) dev_info(>mmc->class_dev, "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n", slot->id, host->bus_hz, clock, @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) mci_send_cmd(slot, sdmmc_cmd_bits, 0); /* keep the clock with reflecting clock dividor */ - slot->__clk_old = clock << div; + slot->__clk_old = (div < 32) ? (clock << div) : 0; } host->current_speed = clock; -- 1.7.4.1
[PATCH] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus(). The warnings are caused because of shift with more than 31 on 32 bit variable, so this patch fixes to shift only for less than 32. UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14 shift exponent 250 is too large for 32-bit type 'unsigned int' Call trace: [] dump_backtrace+0x0/0x380 [] show_stack+0x14/0x20 [] dump_stack+0xe0/0x120 [] ubsan_epilogue+0x18/0x68 [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc [] dw_mci_setup_bus+0x3a0/0x438 [...] UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27 shift exponent 250 is too large for 32-bit type 'unsigned int' Call trace: [] dump_backtrace+0x0/0x380 [] show_stack+0x14/0x20 [] dump_stack+0xe0/0x120 [] ubsan_epilogue+0x18/0x68 [] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc [] dw_mci_setup_bus+0x384/0x438 [] dw_mci_set_ios+0x184/0x798 [] mmc_power_up+0x11c/0x260 [] mmc_start_host+0x88/0x100 [] mmc_add_host+0x6c/0x128 [] dw_mci_probe+0x1088/0x1750 [] dw_mci_pltfm_register+0x108/0x178 [] dw_mci_exynos_probe+0x4c/0x88 [] platform_drv_probe+0x78/0x180 [] driver_probe_device+0x144/0x460 [] __driver_attach+0xf4/0x140 [] bus_for_each_dev+0xf0/0x160 [] driver_attach+0x34/0x58 [] bus_add_driver+0x2c0/0x398 [] driver_register+0xbc/0x1e0 [] __platform_driver_register+0x84/0xa8 [] dw_mci_exynos_pltfm_driver_init+0x18/0x20 [] do_one_initcall+0xa0/0x2c8 [] kernel_init_freeable+0x52c/0x5dc [] kernel_init+0x1c/0xf8 [] ret_from_fork+0x10/0x40 Signed-off-by: Seung-Woo Kim --- drivers/mmc/host/dw_mmc.c |5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c index 2cc6123..dff045e 100644 --- a/drivers/mmc/host/dw_mmc.c +++ b/drivers/mmc/host/dw_mmc.c @@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0; - if ((clock << div) != slot->__clk_old || force_clkinit) + if (((div < 32) ? (clock << div) : 0) != slot->__clk_old || + force_clkinit) dev_info(>mmc->class_dev, "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ div = %d)\n", slot->id, host->bus_hz, clock, @@ -1129,7 +1130,7 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit) mci_send_cmd(slot, sdmmc_cmd_bits, 0); /* keep the clock with reflecting clock dividor */ - slot->__clk_old = clock << div; + slot->__clk_old = (div < 32) ? (clock << div) : 0; } host->current_speed = clock; -- 1.7.4.1
Re: [BUG] Panic when systemd boot do mkdir on tmpfs mounted path with smack enabled environment
Hello, On 2016년 05월 28일 07:44, Casey Schaufler wrote: > On 5/27/2016 1:24 PM, Al Viro wrote: >> On Fri, May 27, 2016 at 12:48:23PM -0700, Linus Torvalds wrote: >>> On Fri, May 27, 2016 at 12:43 PM, Al Viro <v...@zeniv.linux.org.uk> wrote: >>>> Amended and force-pushed... >>> Ok, I'll ignore that branch for now, in the hopes that there will be >>> actual testing success. Please send a full pull request at that point, >>> ok? >> Sure. FWIW, the original bug report had been from +0900, so it's what, >> about 5am Saturday morning there? Let's hope somebody will be there >> today, weekend or not... >> > Al, I have verified that upstream has the problem and that > your vfs.git smack-fix branch works correctly. > It seems a bit late, but it works without reported panic for my environment too. Thanks, - Seung-Woo Kim -- Seung-Woo Kim Samsung Software R Center --
Re: [BUG] Panic when systemd boot do mkdir on tmpfs mounted path with smack enabled environment
Hello, On 2016년 05월 28일 07:44, Casey Schaufler wrote: > On 5/27/2016 1:24 PM, Al Viro wrote: >> On Fri, May 27, 2016 at 12:48:23PM -0700, Linus Torvalds wrote: >>> On Fri, May 27, 2016 at 12:43 PM, Al Viro wrote: >>>> Amended and force-pushed... >>> Ok, I'll ignore that branch for now, in the hopes that there will be >>> actual testing success. Please send a full pull request at that point, >>> ok? >> Sure. FWIW, the original bug report had been from +0900, so it's what, >> about 5am Saturday morning there? Let's hope somebody will be there >> today, weekend or not... >> > Al, I have verified that upstream has the problem and that > your vfs.git smack-fix branch works correctly. > It seems a bit late, but it works without reported panic for my environment too. Thanks, - Seung-Woo Kim -- Seung-Woo Kim Samsung Software R Center --
[BUG] Panic when systemd boot do mkdir on tmpfs mounted path with smack enabled environment
Hello,
After commit, "b968091 security_d_instantiate(): move to the point prior to
attaching dentry to inode", booting on system with
systemd and security smack, following kernel panic occurs.
---
Unable to handle kernel paging request at virtual address fff4
pgd = eda74000
[fff4] *pgd=6fffd861, *pte=, *ppte=
Internal error: Oops: 37 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: systemd Not tainted 4.6.0-11010-gdc03c0f-dirty #54
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
task: ee948000 ti: ee942000 task.ti: ee942000
PC is at do_raw_spin_lock+0x14/0x1c0
LR is at _raw_spin_lock+0x28/0x2c
pc : []lr : []psr: 000f0013
sp : ee943d98 ip : ee943dc0 fp : ee943dbc
r10: r9 : ed8a1f80 r8 : fff0
r7 : eea57d40 r6 : c0d5c764 r5 : ffe8 r4 : fff0
r3 : ee948000 r2 : 0001 r1 : c0d5c77e r0 : fff0
Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 6da7406a DAC: 0051
Process systemd (pid: 1, stack limit = 0xee942210)
Stack: (0xee943d98 to 0xee944000)
3d80: fff0 ffe8
3da0: c0d5c764 eea57d40 fff0 ed8a1f80 ee943dd4 ee943dc0 c0a9f608 c016e694
3dc0: 0004 ee943dfc ee943dd8 c0271bf8 c0a9f5ec c0d5c7ec
3de0: ed5f9428 ee314480 ed8a1f80 ee943e24 ee943e00 c0202e40 c0271ba4
3e00: c0270d28 0004 ed5f9428 c0d5c7ec 0004 ee943e54 ee943e28
3e20: c0270d54 c0202e08 0004 0100 c0d5c76d eda8d380 eda8d380
3e40: eea51430 eda8d38c ee943e9c ee943e58 c03aa4c0 c0270cec ed5f9440
3e60: c114a3c8 0004 ee943edc ee943e78 c03a4e60 c114ad70 c114a678 eea51430
3e80: ed5f9428 ff9c ee943ebc ee943ea0 c03a4d84 c03aa1c4
3ea0: eea51430 ed5f9428 eea51430 ed5f9428 ee943edc ee943ec0 c0262c18 c03a4d4c
3ec0: eea507d0 eea50780 eea51430 ee943f1c ee943ee0 c020363c c0262bf8
3ee0: c01014c4 386d439a 35a4e900 c02038cc 01ed 01ed
3f00: eea50780 ed5f9428 7f65f7f8 ee943f34 ee943f20 c02038f0 c0203568
3f20: 01ed eea50780 ee943f64 ee943f38 c0255fbc c02038d8 ee3a4015
3f40: 01ed 01ed ed5f9428 7f65f7f8 0002 01ed ee943f94 ee943f68
3f60: c025a1ec c0255f08 eda96310 ed5f8d10 01ed 7f65f7f8 003520a2 0027
3f80: c0109248 ee942000 ee943fa4 ee943f98 c025a25c c025a180 ee943fa8
3fa0: c0109040 c025a244 01ed 7f65f7f8 7f65f7f8 01ed 00104000
3fc0: 01ed 7f65f7f8 003520a2 0027 b6f9beb0 7f6bb9b0 bebe3998 bebe3988
3fe0: 7f6bb9e8 bebe387c 7f60419d b6df434c 600f0010 7f65f7f8 fffb
[] (do_raw_spin_lock) from [] (_raw_spin_lock+0x28/0x2c)
[] (_raw_spin_lock) from [] (simple_xattr_set+0x60/0x158)
[] (simple_xattr_set) from []
(shmem_xattr_handler_set+0x44/0x4c)
[] (shmem_xattr_handler_set) from []
(generic_setxattr+0x74/0x7c)
[] (generic_setxattr) from []
(smack_d_instantiate+0x308/0x390)
[] (smack_d_instantiate) from []
(security_d_instantiate+0x44/0x64)
[] (security_d_instantiate) from []
(d_instantiate+0x2c/0x5c)
[] (d_instantiate) from [] (shmem_mknod+0xe0/0xfc)
[] (shmem_mknod) from [] (shmem_mkdir+0x24/0x3c)
[] (shmem_mkdir) from [] (vfs_mkdir+0xc0/0x10c)
[] (vfs_mkdir) from [] (SyS_mkdirat+0x78/0xc4)
[] (SyS_mkdirat) from [] (SyS_mkdir+0x24/0x28)
[] (SyS_mkdir) from [] (ret_fast_syscall+0x0/0x3c)
Code: e92ddbf0 e24cb004 e52de004 e8bd4000 (e5902004)
---[ end trace ec9873d14ae12b14 ]---
It works fine if reverting the commit, "b968091 security_d_instantiate(): move
to the point prior to attaching dentry to inode", for
d_instantiate() like following.
---
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1793,11 +1793,11 @@ void d_instantiate(struct dentry *entry, struct inode *
inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
if (inode) {
- security_d_instantiate(entry, inode);
spin_lock(>i_lock);
__d_instantiate(entry, inode);
spin_unlock(>i_lock);
}
+ security_d_instantiate(entry, inode);
}
EXPORT_SYMBOL(d_instantiate);
---
In my test environment, following related configs are enabled.
CONFIG_SHMEM=y
CONFIG_TMPFS=y
CONFIG_TMPFS_POSIX_ACL=y
CONFIG_TMPFS_XATTR=y
CONFIG_SECURITY_SMACK=y
CONFIG_DEFAULT_SECURITY_SMACK=y
CONFIG_DEFAULT_SECURITY="smack"
Best Regards,
- Seung-Woo Kim
[BUG] Panic when systemd boot do mkdir on tmpfs mounted path with smack enabled environment
Hello,
After commit, "b968091 security_d_instantiate(): move to the point prior to
attaching dentry to inode", booting on system with
systemd and security smack, following kernel panic occurs.
---
Unable to handle kernel paging request at virtual address fff4
pgd = eda74000
[fff4] *pgd=6fffd861, *pte=, *ppte=
Internal error: Oops: 37 [#1] PREEMPT SMP ARM
Modules linked in:
CPU: 0 PID: 1 Comm: systemd Not tainted 4.6.0-11010-gdc03c0f-dirty #54
Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
task: ee948000 ti: ee942000 task.ti: ee942000
PC is at do_raw_spin_lock+0x14/0x1c0
LR is at _raw_spin_lock+0x28/0x2c
pc : []lr : []psr: 000f0013
sp : ee943d98 ip : ee943dc0 fp : ee943dbc
r10: r9 : ed8a1f80 r8 : fff0
r7 : eea57d40 r6 : c0d5c764 r5 : ffe8 r4 : fff0
r3 : ee948000 r2 : 0001 r1 : c0d5c77e r0 : fff0
Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
Control: 10c5387d Table: 6da7406a DAC: 0051
Process systemd (pid: 1, stack limit = 0xee942210)
Stack: (0xee943d98 to 0xee944000)
3d80: fff0 ffe8
3da0: c0d5c764 eea57d40 fff0 ed8a1f80 ee943dd4 ee943dc0 c0a9f608 c016e694
3dc0: 0004 ee943dfc ee943dd8 c0271bf8 c0a9f5ec c0d5c7ec
3de0: ed5f9428 ee314480 ed8a1f80 ee943e24 ee943e00 c0202e40 c0271ba4
3e00: c0270d28 0004 ed5f9428 c0d5c7ec 0004 ee943e54 ee943e28
3e20: c0270d54 c0202e08 0004 0100 c0d5c76d eda8d380 eda8d380
3e40: eea51430 eda8d38c ee943e9c ee943e58 c03aa4c0 c0270cec ed5f9440
3e60: c114a3c8 0004 ee943edc ee943e78 c03a4e60 c114ad70 c114a678 eea51430
3e80: ed5f9428 ff9c ee943ebc ee943ea0 c03a4d84 c03aa1c4
3ea0: eea51430 ed5f9428 eea51430 ed5f9428 ee943edc ee943ec0 c0262c18 c03a4d4c
3ec0: eea507d0 eea50780 eea51430 ee943f1c ee943ee0 c020363c c0262bf8
3ee0: c01014c4 386d439a 35a4e900 c02038cc 01ed 01ed
3f00: eea50780 ed5f9428 7f65f7f8 ee943f34 ee943f20 c02038f0 c0203568
3f20: 01ed eea50780 ee943f64 ee943f38 c0255fbc c02038d8 ee3a4015
3f40: 01ed 01ed ed5f9428 7f65f7f8 0002 01ed ee943f94 ee943f68
3f60: c025a1ec c0255f08 eda96310 ed5f8d10 01ed 7f65f7f8 003520a2 0027
3f80: c0109248 ee942000 ee943fa4 ee943f98 c025a25c c025a180 ee943fa8
3fa0: c0109040 c025a244 01ed 7f65f7f8 7f65f7f8 01ed 00104000
3fc0: 01ed 7f65f7f8 003520a2 0027 b6f9beb0 7f6bb9b0 bebe3998 bebe3988
3fe0: 7f6bb9e8 bebe387c 7f60419d b6df434c 600f0010 7f65f7f8 fffb
[] (do_raw_spin_lock) from [] (_raw_spin_lock+0x28/0x2c)
[] (_raw_spin_lock) from [] (simple_xattr_set+0x60/0x158)
[] (simple_xattr_set) from []
(shmem_xattr_handler_set+0x44/0x4c)
[] (shmem_xattr_handler_set) from []
(generic_setxattr+0x74/0x7c)
[] (generic_setxattr) from []
(smack_d_instantiate+0x308/0x390)
[] (smack_d_instantiate) from []
(security_d_instantiate+0x44/0x64)
[] (security_d_instantiate) from []
(d_instantiate+0x2c/0x5c)
[] (d_instantiate) from [] (shmem_mknod+0xe0/0xfc)
[] (shmem_mknod) from [] (shmem_mkdir+0x24/0x3c)
[] (shmem_mkdir) from [] (vfs_mkdir+0xc0/0x10c)
[] (vfs_mkdir) from [] (SyS_mkdirat+0x78/0xc4)
[] (SyS_mkdirat) from [] (SyS_mkdir+0x24/0x28)
[] (SyS_mkdir) from [] (ret_fast_syscall+0x0/0x3c)
Code: e92ddbf0 e24cb004 e52de004 e8bd4000 (e5902004)
---[ end trace ec9873d14ae12b14 ]---
It works fine if reverting the commit, "b968091 security_d_instantiate(): move
to the point prior to attaching dentry to inode", for
d_instantiate() like following.
---
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -1793,11 +1793,11 @@ void d_instantiate(struct dentry *entry, struct inode *
inode)
{
BUG_ON(!hlist_unhashed(>d_u.d_alias));
if (inode) {
- security_d_instantiate(entry, inode);
spin_lock(>i_lock);
__d_instantiate(entry, inode);
spin_unlock(>i_lock);
}
+ security_d_instantiate(entry, inode);
}
EXPORT_SYMBOL(d_instantiate);
---
In my test environment, following related configs are enabled.
CONFIG_SHMEM=y
CONFIG_TMPFS=y
CONFIG_TMPFS_POSIX_ACL=y
CONFIG_TMPFS_XATTR=y
CONFIG_SECURITY_SMACK=y
CONFIG_DEFAULT_SECURITY_SMACK=y
CONFIG_DEFAULT_SECURITY="smack"
Best Regards,
- Seung-Woo Kim
Re: [PATCH 0/3] drm/exynos: Kconfig dependency fixes
Hello Javier,
On 2016년 03월 29일 11:41, Javier Martinez Canillas wrote:
> Hello Seung-Woo,
>
> Thanks a lot for your feedback.
>
> On 03/28/2016 09:46 PM, Seung-Woo Kim wrote:
>> Hi Javier,
>>
>> On 2016년 03월 29일 10:28, Javier Martinez Canillas wrote:
>>> Hello Inki,
>>>
>>> This patch series contains some fixes for the Kconfig symbol dependencies
>>> of the Exynos DRM driver. They make sure that the Exynos DRM components
>>> and the media platform drivers that makes use of the same HW IP block are
>>> not enabled at the same time.
>>>
>>> Best regards,
>>> Javier
>>>
>>>
>>> Javier Martinez Canillas (3):
>>> drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>>> drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>>> drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
>>
>> In G2D case, there is only one instance, but for the other cases, there
>> are several instances and in my environment, I enable both drivers on
>> v4l2 and drm FIMC/GSC.
>>
>> So, IMHO, the not-enabled v4l2 dependency is not really required for drm
>> fimc and drm gsc.
>>
>
> I'm confused, it was you who added the depends on !VIDEO_SAMSUNG_EXYNOS_GSC
> for DRM_EXYNOS_GSC in commit aeefb36832e5 ("drm/exynos: gsc: add device tree
> support and remove usage of static mappings").
Yes, you are right. Originally, my goal on the GSC was bringing optional
flag for setting isp mode or wb mode like FIMC in tizen.org git tree.
https://review.tizen.org/git/?p=platform/kernel/linux-exynos.git;a=blobdiff;f=Documentation/devicetree/bindings/media/exynos5-gsc.txt;h=d526777a3abd04d244c46fdd729e3df93bb86917;hp=0604d42f38d1941526d47ad11a958a2a83797f97;hb=751cd6d88d9620c83042641b52fdd244408a3947;hpb=7c7ab44f86d64ba6a6733da8f201a26a6c0e807f
But only devicetree part of GSC was upstreamed and simultaneous enabling
both v4l2 gsc and drm gsc driver is removed. The only reason I set both
driver simultaneously was enabling video codec, exynos-mfc with gsc to
convert RGB plane.
I know Marek already has plan to integrate yuv plane feature of GSC to
DRM KMS. Also, for GSC, simultaneous setup is alredy remove, so your
patch seems better.
>
>>From the commit message "The driver cannot be used simultaneously with V4L2
> Mem2Mem GScaller driver thought". Did that assumption changed and the depend
> should be removed then? or maybe I misunderstood what you meant.
>
> Now, I'm not really sure about FIMC either, it was feedback I got from this
> patch [0]. Could you please take a look to that and let me know if enabling
> these drivers simultaneously makes sense then?
About FIMC, there is still simultaneous setup for both v4l2 and drm
driver with devicetree binding flags, samsung,isp-wb and samsung,lcd-wb.
If on the FIMC instance of dt node, samsung,lcd-wb is set, then drm
driver is probed, otherwise v4l2 driver is probed.
Simultaneous FIMC driver is also only for RGB converting of video codec
planes like GSC at least to me.
Marek, do you have any idea about the simultaneous setup for fimc and gsc?
Best Regards,
- Seung-Woo Kim
>
>> Best Regards,
>> - Seung-Woo Kim
>>
>>>
>>> drivers/gpu/drm/exynos/Kconfig | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>
> [0]: https://lkml.org/lkml/2016/3/23/292
>
> Best regards,
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH 0/3] drm/exynos: Kconfig dependency fixes
Hello Javier,
On 2016년 03월 29일 11:41, Javier Martinez Canillas wrote:
> Hello Seung-Woo,
>
> Thanks a lot for your feedback.
>
> On 03/28/2016 09:46 PM, Seung-Woo Kim wrote:
>> Hi Javier,
>>
>> On 2016년 03월 29일 10:28, Javier Martinez Canillas wrote:
>>> Hello Inki,
>>>
>>> This patch series contains some fixes for the Kconfig symbol dependencies
>>> of the Exynos DRM driver. They make sure that the Exynos DRM components
>>> and the media platform drivers that makes use of the same HW IP block are
>>> not enabled at the same time.
>>>
>>> Best regards,
>>> Javier
>>>
>>>
>>> Javier Martinez Canillas (3):
>>> drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency
>>> drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency
>>> drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n
>>
>> In G2D case, there is only one instance, but for the other cases, there
>> are several instances and in my environment, I enable both drivers on
>> v4l2 and drm FIMC/GSC.
>>
>> So, IMHO, the not-enabled v4l2 dependency is not really required for drm
>> fimc and drm gsc.
>>
>
> I'm confused, it was you who added the depends on !VIDEO_SAMSUNG_EXYNOS_GSC
> for DRM_EXYNOS_GSC in commit aeefb36832e5 ("drm/exynos: gsc: add device tree
> support and remove usage of static mappings").
Yes, you are right. Originally, my goal on the GSC was bringing optional
flag for setting isp mode or wb mode like FIMC in tizen.org git tree.
https://review.tizen.org/git/?p=platform/kernel/linux-exynos.git;a=blobdiff;f=Documentation/devicetree/bindings/media/exynos5-gsc.txt;h=d526777a3abd04d244c46fdd729e3df93bb86917;hp=0604d42f38d1941526d47ad11a958a2a83797f97;hb=751cd6d88d9620c83042641b52fdd244408a3947;hpb=7c7ab44f86d64ba6a6733da8f201a26a6c0e807f
But only devicetree part of GSC was upstreamed and simultaneous enabling
both v4l2 gsc and drm gsc driver is removed. The only reason I set both
driver simultaneously was enabling video codec, exynos-mfc with gsc to
convert RGB plane.
I know Marek already has plan to integrate yuv plane feature of GSC to
DRM KMS. Also, for GSC, simultaneous setup is alredy remove, so your
patch seems better.
>
>>From the commit message "The driver cannot be used simultaneously with V4L2
> Mem2Mem GScaller driver thought". Did that assumption changed and the depend
> should be removed then? or maybe I misunderstood what you meant.
>
> Now, I'm not really sure about FIMC either, it was feedback I got from this
> patch [0]. Could you please take a look to that and let me know if enabling
> these drivers simultaneously makes sense then?
About FIMC, there is still simultaneous setup for both v4l2 and drm
driver with devicetree binding flags, samsung,isp-wb and samsung,lcd-wb.
If on the FIMC instance of dt node, samsung,lcd-wb is set, then drm
driver is probed, otherwise v4l2 driver is probed.
Simultaneous FIMC driver is also only for RGB converting of video codec
planes like GSC at least to me.
Marek, do you have any idea about the simultaneous setup for fimc and gsc?
Best Regards,
- Seung-Woo Kim
>
>> Best Regards,
>> - Seung-Woo Kim
>>
>>>
>>> drivers/gpu/drm/exynos/Kconfig | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>
>
> [0]: https://lkml.org/lkml/2016/3/23/292
>
> Best regards,
>
--
Seung-Woo Kim
Samsung Software R Center
--
Re: [PATCH 0/3] drm/exynos: Kconfig dependency fixes
Hi Javier, On 2016년 03월 29일 10:28, Javier Martinez Canillas wrote: > Hello Inki, > > This patch series contains some fixes for the Kconfig symbol dependencies > of the Exynos DRM driver. They make sure that the Exynos DRM components > and the media platform drivers that makes use of the same HW IP block are > not enabled at the same time. > > Best regards, > Javier > > > Javier Martinez Canillas (3): > drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency > drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency > drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n In G2D case, there is only one instance, but for the other cases, there are several instances and in my environment, I enable both drivers on v4l2 and drm FIMC/GSC. So, IMHO, the not-enabled v4l2 dependency is not really required for drm fimc and drm gsc. Best Regards, - Seung-Woo Kim > > drivers/gpu/drm/exynos/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > -- Seung-Woo Kim Samsung Software R Center --
Re: [PATCH 0/3] drm/exynos: Kconfig dependency fixes
Hi Javier, On 2016년 03월 29일 10:28, Javier Martinez Canillas wrote: > Hello Inki, > > This patch series contains some fixes for the Kconfig symbol dependencies > of the Exynos DRM driver. They make sure that the Exynos DRM components > and the media platform drivers that makes use of the same HW IP block are > not enabled at the same time. > > Best regards, > Javier > > > Javier Martinez Canillas (3): > drm/exynos: Use VIDEO_SAMSUNG_S5P_G2D=n as G2D Kconfig dependency > drm/exynos: Use VIDEO_SAMSUNG_EXYNOS_GSC=n as GSC Kconfig dependency > drm/exynos: Make DRM_EXYNOS_FIMC depend on VIDEO_S5P_FIMC=n In G2D case, there is only one instance, but for the other cases, there are several instances and in my environment, I enable both drivers on v4l2 and drm FIMC/GSC. So, IMHO, the not-enabled v4l2 dependency is not really required for drm fimc and drm gsc. Best Regards, - Seung-Woo Kim > > drivers/gpu/drm/exynos/Kconfig | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > -- Seung-Woo Kim Samsung Software R Center --
Re: [RESEND PATCH] Smack: ignore private inode for smack_file_receive
Ping? On 2015년 04월 17일 15:25, Seung-Woo Kim wrote: > The dmabuf fd can be shared between processes via unix domain > socket. The file of dmabuf fd is came from anon_inode. The inode > has no set and get xattr operations, so it can not be shared > between processes with smack. This patch fixes just to ignore > private inode including anon_inode for smack_file_receive. > > Signed-off-by: Seung-Woo Kim > --- > I think there was some issue sending my previous mail, so I resend patch > again. > --- > security/smack/smack_lsm.c |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c > index 69fdc38..d1bb411 100644 > --- a/security/smack/smack_lsm.c > +++ b/security/smack/smack_lsm.c > @@ -1673,6 +1673,9 @@ static int smack_file_receive(struct file *file) > struct smk_audit_info ad; > struct inode *inode = file_inode(file); > > + if (unlikely(IS_PRIVATE(inode))) > + return 0; > + > smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH); > smk_ad_setfield_u_fs_path(, file->f_path); > /* > -- Seung-Woo Kim Samsung Software R Center -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RESEND PATCH] Smack: ignore private inode for smack_file_receive
Ping? On 2015년 04월 17일 15:25, Seung-Woo Kim wrote: The dmabuf fd can be shared between processes via unix domain socket. The file of dmabuf fd is came from anon_inode. The inode has no set and get xattr operations, so it can not be shared between processes with smack. This patch fixes just to ignore private inode including anon_inode for smack_file_receive. Signed-off-by: Seung-Woo Kim sw0312@samsung.com --- I think there was some issue sending my previous mail, so I resend patch again. --- security/smack/smack_lsm.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 69fdc38..d1bb411 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1673,6 +1673,9 @@ static int smack_file_receive(struct file *file) struct smk_audit_info ad; struct inode *inode = file_inode(file); + if (unlikely(IS_PRIVATE(inode))) + return 0; + smk_ad_init(ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(ad, file-f_path); /* -- Seung-Woo Kim Samsung Software RD Center -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH] Smack: ignore private inode for smack_file_receive
The dmabuf fd can be shared between processes via unix domain socket. The file of dmabuf fd is came from anon_inode. The inode has no set and get xattr operations, so it can not be shared between processes with smack. This patch fixes just to ignore private inode including anon_inode for smack_file_receive. Signed-off-by: Seung-Woo Kim --- I think there was some issue sending my previous mail, so I resend patch again. --- security/smack/smack_lsm.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 69fdc38..d1bb411 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1673,6 +1673,9 @@ static int smack_file_receive(struct file *file) struct smk_audit_info ad; struct inode *inode = file_inode(file); + if (unlikely(IS_PRIVATE(inode))) + return 0; + smk_ad_init(, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(, file->f_path); /* -- 1.7.4.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[RESEND PATCH] Smack: ignore private inode for smack_file_receive
The dmabuf fd can be shared between processes via unix domain socket. The file of dmabuf fd is came from anon_inode. The inode has no set and get xattr operations, so it can not be shared between processes with smack. This patch fixes just to ignore private inode including anon_inode for smack_file_receive. Signed-off-by: Seung-Woo Kim sw0312@samsung.com --- I think there was some issue sending my previous mail, so I resend patch again. --- security/smack/smack_lsm.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c index 69fdc38..d1bb411 100644 --- a/security/smack/smack_lsm.c +++ b/security/smack/smack_lsm.c @@ -1673,6 +1673,9 @@ static int smack_file_receive(struct file *file) struct smk_audit_info ad; struct inode *inode = file_inode(file); + if (unlikely(IS_PRIVATE(inode))) + return 0; + smk_ad_init(ad, __func__, LSM_AUDIT_DATA_PATH); smk_ad_setfield_u_fs_path(ad, file-f_path); /* -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Security: ignore private inode from security_file_receive
Hello,
On 2015년 04월 16일 22:48, Stephen Smalley wrote:
> On 04/16/2015 09:40 AM, Seung-Woo Kim wrote:
>> The dma-buf fd from anon_inode can be shared across processes, but
>> there is no way to set security permission for the fd. So this
>> patch fix just to ignore private inode from security_file_receive.
>>
>> Signed-off-by: Seung-Woo Kim
>> ---
>>
>> If security like smack is enabled, the dmabuf fd can not be shared between
>> processes via unix domain socket. I am not familiar with security, so I am
>> not sure that this kind of patch can be acceptable.
>>
>> Is there other option to share dmabuf fd via socket with security check?
>>
>> Best Regards,
>> - Seung-Woo Kim
>>
>> ---
>> security/security.c |3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/security/security.c b/security/security.c
>> index 730ac65..c57354c 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -810,6 +810,9 @@ int security_file_send_sigiotask(struct task_struct *tsk,
>>
>> int security_file_receive(struct file *file)
>> {
>> +
>> +if (unlikely(IS_PRIVATE(file->f_path.dentry->d_inode)))
>> +return 0;
>> return security_ops->file_receive(file);
>> }
>
> SELinux handles this internally; see its inode_has_perm() function.
> Doing it here would prevent any security module checking at all, even of
> the struct file, which SELinux does presently do (selinux_file_receive
> calls file_has_perm which applies the fd use check and then calls
> inode_has_perm on the inode). Unless you are saying that the
> file->f_security field is also not being set correctly.
Thanks for the suggestion. I will try to do on smack side.
Best Regards,
- Seung-Woo Kim
>
>
>
--
Seung-Woo Kim
Samsung Software R Center
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] Security: ignore private inode from security_file_receive
The dma-buf fd from anon_inode can be shared across processes, but
there is no way to set security permission for the fd. So this
patch fix just to ignore private inode from security_file_receive.
Signed-off-by: Seung-Woo Kim
---
If security like smack is enabled, the dmabuf fd can not be shared between
processes via unix domain socket. I am not familiar with security, so I am
not sure that this kind of patch can be acceptable.
Is there other option to share dmabuf fd via socket with security check?
Best Regards,
- Seung-Woo Kim
---
security/security.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/security/security.c b/security/security.c
index 730ac65..c57354c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -810,6 +810,9 @@ int security_file_send_sigiotask(struct task_struct *tsk,
int security_file_receive(struct file *file)
{
+
+ if (unlikely(IS_PRIVATE(file->f_path.dentry->d_inode)))
+ return 0;
return security_ops->file_receive(file);
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[RFC PATCH] Security: ignore private inode from security_file_receive
The dma-buf fd from anon_inode can be shared across processes, but
there is no way to set security permission for the fd. So this
patch fix just to ignore private inode from security_file_receive.
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
---
If security like smack is enabled, the dmabuf fd can not be shared between
processes via unix domain socket. I am not familiar with security, so I am
not sure that this kind of patch can be acceptable.
Is there other option to share dmabuf fd via socket with security check?
Best Regards,
- Seung-Woo Kim
---
security/security.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/security/security.c b/security/security.c
index 730ac65..c57354c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -810,6 +810,9 @@ int security_file_send_sigiotask(struct task_struct *tsk,
int security_file_receive(struct file *file)
{
+
+ if (unlikely(IS_PRIVATE(file-f_path.dentry-d_inode)))
+ return 0;
return security_ops-file_receive(file);
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH] Security: ignore private inode from security_file_receive
Hello,
On 2015년 04월 16일 22:48, Stephen Smalley wrote:
On 04/16/2015 09:40 AM, Seung-Woo Kim wrote:
The dma-buf fd from anon_inode can be shared across processes, but
there is no way to set security permission for the fd. So this
patch fix just to ignore private inode from security_file_receive.
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
---
If security like smack is enabled, the dmabuf fd can not be shared between
processes via unix domain socket. I am not familiar with security, so I am
not sure that this kind of patch can be acceptable.
Is there other option to share dmabuf fd via socket with security check?
Best Regards,
- Seung-Woo Kim
---
security/security.c |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/security/security.c b/security/security.c
index 730ac65..c57354c 100644
--- a/security/security.c
+++ b/security/security.c
@@ -810,6 +810,9 @@ int security_file_send_sigiotask(struct task_struct *tsk,
int security_file_receive(struct file *file)
{
+
+if (unlikely(IS_PRIVATE(file-f_path.dentry-d_inode)))
+return 0;
return security_ops-file_receive(file);
}
SELinux handles this internally; see its inode_has_perm() function.
Doing it here would prevent any security module checking at all, even of
the struct file, which SELinux does presently do (selinux_file_receive
calls file_has_perm which applies the fd use check and then calls
inode_has_perm on the inode). Unless you are saying that the
file-f_security field is also not being set correctly.
Thanks for the suggestion. I will try to do on smack side.
Best Regards,
- Seung-Woo Kim
--
Seung-Woo Kim
Samsung Software RD Center
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4
Hello, On 2015년 03월 25일 03:32, Rob Clark wrote: > On Mon, Mar 16, 2015 at 4:05 AM, Daniel Vetter wrote: >> On Fri, Mar 13, 2015 at 03:12:10PM -0400, Stephane Viau wrote: >>> From: Beeresh Gopal >>> >>> Using fb modifier flag, support NV12MT format in MDP4. >>> >>> v2: >>> - rework the modifier's description [Daniel Vetter's comment] >>> - drop .set_mode_config() callback [Rob Clark's comment] >>> >>> Signed-off-by: Beeresh Gopal >>> Signed-off-by: Stephane Viau >>> --- >>> drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 2 ++ >>> drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 22 ++ >>> include/uapi/drm/drm_fourcc.h | 5 + >>> 3 files changed, 29 insertions(+) >>> >>> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h >>> index 188e61f..2ff79cb 100644 >>> --- a/include/uapi/drm/drm_fourcc.h >>> +++ b/include/uapi/drm/drm_fourcc.h >>> @@ -161,4 +161,9 @@ >>> * authoritative source for all of these. >>> */ >>> >>> +/* Samsung framebuffer modifiers */ >>> + >>> +/* Tiled: 64x32 pixel macroblocks */ >> >> Since this seems shared by a lot of vendors (I still don't believe Samsung >> invented this really ...) can you please describe this thing a bit in more >> detail? Somewhat important how macroblocks are laid out and pixels within. >> Also with a planar format like NV12 "pixel" is a bit unclear, e.g. what >> happens if you throw 10bit plane formats at this? So maybe also add a note >> that for now this is only used together with NV12T. > > + a couple folks from Samsung, since I expect they want this for > exynos as well (and might be able to help with the description) Yes, I have a plan to apply fb_modifier for exynos with kms interface of hdmi. > > vl4 also has this format, but last I looked was rather light on the details. I am not sure msm mdp uses exactly same format with exynos, but anyway v4l2 NV12MT format was introduced for exynos hw video codec. macro blocks for the format is laid z-order and each pixel data in each macro block is just normal NV12 style. I think Marek and Sylwester can help understanding the format. > > http://linuxtv.org/downloads/v4l-dvb-apis/re31.html > > I know up in userspace, GStreamer seems to have some support for this format: > > http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f8d3b9b4fcc5e08b771314fa95e9ed8f750b54e6 > >> Then there's the question of validating the input - stride should probably >> be a full multiple of the macroblock size. Since this is a shared format >> imo this kind of checking should be done in drm core. > > afaiu, stride (and maybe even width?) should be a multiple of the > block size (but height does not) > > BR, > -R > > >> -Daniel >> >> >>> +#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1) >>> + >>> #endif /* DRM_FOURCC_H */ >>> -- >>> Qualcomm Innovation Center, Inc. >>> >>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, >>> a Linux Foundation Collaborative Project >>> >> >> -- >> Daniel Vetter >> Software Engineer, Intel Corporation >> +41 (0) 79 365 57 48 - http://blog.ffwll.ch > -- Seung-Woo Kim Samsung Software R Center -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 2/2] drm/msm/mdp4: Support NV12MT format in mdp4
Hello, On 2015년 03월 25일 03:32, Rob Clark wrote: On Mon, Mar 16, 2015 at 4:05 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Mar 13, 2015 at 03:12:10PM -0400, Stephane Viau wrote: From: Beeresh Gopal gbeer...@codeaurora.org Using fb modifier flag, support NV12MT format in MDP4. v2: - rework the modifier's description [Daniel Vetter's comment] - drop .set_mode_config() callback [Rob Clark's comment] Signed-off-by: Beeresh Gopal gbeer...@codeaurora.org Signed-off-by: Stephane Viau sv...@codeaurora.org --- drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c | 2 ++ drivers/gpu/drm/msm/mdp/mdp4/mdp4_plane.c | 22 ++ include/uapi/drm/drm_fourcc.h | 5 + 3 files changed, 29 insertions(+) snip. diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 188e61f..2ff79cb 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -161,4 +161,9 @@ * authoritative source for all of these. */ +/* Samsung framebuffer modifiers */ + +/* Tiled: 64x32 pixel macroblocks */ Since this seems shared by a lot of vendors (I still don't believe Samsung invented this really ...) can you please describe this thing a bit in more detail? Somewhat important how macroblocks are laid out and pixels within. Also with a planar format like NV12 pixel is a bit unclear, e.g. what happens if you throw 10bit plane formats at this? So maybe also add a note that for now this is only used together with NV12T. + a couple folks from Samsung, since I expect they want this for exynos as well (and might be able to help with the description) Yes, I have a plan to apply fb_modifier for exynos with kms interface of hdmi. vl4 also has this format, but last I looked was rather light on the details. I am not sure msm mdp uses exactly same format with exynos, but anyway v4l2 NV12MT format was introduced for exynos hw video codec. macro blocks for the format is laid z-order and each pixel data in each macro block is just normal NV12 style. I think Marek and Sylwester can help understanding the format. http://linuxtv.org/downloads/v4l-dvb-apis/re31.html I know up in userspace, GStreamer seems to have some support for this format: http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=f8d3b9b4fcc5e08b771314fa95e9ed8f750b54e6 Then there's the question of validating the input - stride should probably be a full multiple of the macroblock size. Since this is a shared format imo this kind of checking should be done in drm core. afaiu, stride (and maybe even width?) should be a multiple of the block size (but height does not) BR, -R -Daniel +#define DRM_FORMAT_MOD_SAMSUNG_64_32_TILE fourcc_mod_code(SAMSUNG, 1) + #endif /* DRM_FOURCC_H */ -- Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- Seung-Woo Kim Samsung Software RD Center -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: Fix regualtor_ena_gpio_free not to access pin after freeing
After freeing pin from regulator_ena_gpio_free, loop can access
the pin. So this patch fixes not to access pin after freeing.
Signed-off-by: Seung-Woo Kim
---
drivers/regulator/core.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cd87c0c..479ef0b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1713,6 +1713,8 @@ static void regulator_ena_gpio_free(struct regulator_dev
*rdev)
gpiod_put(pin->gpiod);
list_del(>list);
kfree(pin);
+ rdev->ena_pin = NULL;
+ return;
} else {
pin->request_count--;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] regulator: core: Fix regualtor_ena_gpio_free not to access pin after freeing
After freeing pin from regulator_ena_gpio_free, loop can access
the pin. So this patch fixes not to access pin after freeing.
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
---
drivers/regulator/core.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index cd87c0c..479ef0b 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1713,6 +1713,8 @@ static void regulator_ena_gpio_free(struct regulator_dev
*rdev)
gpiod_put(pin-gpiod);
list_del(pin-list);
kfree(pin);
+ rdev-ena_pin = NULL;
+ return;
} else {
pin-request_count--;
}
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] drm/exynos/mixer: use MXR_GRP_SXY_SY
Hello Daniel, On 2014년 05월 07일 23:14, Daniel Kurtz wrote: > On Wed, May 7, 2014 at 1:14 PM, Seung-Woo Kim wrote: >> Hi Daniel, >> >> On 2014년 05월 05일 00:26, Daniel Kurtz wrote: >>> Mixer hardware supports offsetting dma from start of source buffer using >>> the MXR_GRP_SXY register. >>> >>> Signed-off-by: Daniel Kurtz >>> --- >>> drivers/gpu/drm/exynos/exynos_mixer.c | 8 +++- >>> 1 file changed, 3 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c >>> b/drivers/gpu/drm/exynos/exynos_mixer.c >>> index 475eb49..40cf39b 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_mixer.c >>> +++ b/drivers/gpu/drm/exynos/exynos_mixer.c >>> @@ -529,13 +529,11 @@ static void mixer_graph_buffer(struct mixer_context >>> *ctx, int win) >>> >>> dst_x_offset = win_data->crtc_x; >>> dst_y_offset = win_data->crtc_y; >>> + src_x_offset = win_data->fb_x; >>> + src_y_offset = win_data->fb_y; >>> >>> /* converting dma address base and source offset */ >>> - dma_addr = win_data->dma_addr >>> - + (win_data->fb_x * win_data->bpp >> 3) >>> - + (win_data->fb_y * win_data->fb_width * win_data->bpp >> 3); >>> - src_x_offset = 0; >>> - src_y_offset = 0; >>> + dma_addr = win_data->dma_addr; >> >> Basically, you are right and source offset register can be used. But >> because of limitation of resolution for mixer up to 1920x1080, I >> considered modified soruce dma address to set one frame buffer, which is >> bigger than 1920x1080, on to both fimd and hdmi. > > Hi Seung-Woo, > > I do not see why the maximum MIXER resolution matters for choosing > between offsetting BASE or using SXY. > > Let's say you have one big 1920x1908 framebuffer, with a span of 1920, > starting at dma_addr (there is no extra padding at the end of the > line). > Let's say you wanted the mixer to scan out 1920x1080 pixels starting > from (0, 800) in the framebuffer, and start drawing them at (0,0) on > the screen. > > What we currently do is: > BASE = dma_addr + (800 * 1080 * 4) > SPAN = 1920 > SXY = SX(0) | SY(0) > WH = W(1920) | H(1080) > DXY = DX(0) | DY(0) > > I am proposing we do: > BASE = dma_addr > SPAN = 1920 > SXY = SX(0) | SY(800) > WH = W(1920) | H(1080) > DXY = DX(0) | DY(0) > > In both cases, the mixer resolution is 1920x1080. In my test to show each half of big one framebuffer (3840 x 1080) to FIMD from 0 to 1079 and MIXER from 1080 to 3839 with exynos4210 and exynos4412, it was failed to show proper hdmi display. Also it is same for framebuffer (1920 x 2160). AFAIK, it is mainly because mixer dma has limitation of dma memory size. In this case, I set register as like: BASE = dma_addr /* 3840 x 1080 x 4 */ SPAN = 3840 SXY = SX(1920) | SY(0) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) or: BASE = dma_addr /* 1920 x 2160 x 4 */ SPAN = 1920 SXY = SX(0) | SY(1080) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) but these two setting did not show hdmi display as I expected. So I used modified dma address. > > My motivation for wanting to program an un-modified dma_addr into BASE > is so we can then just check BASE_S to determine from which buffer the > mixer is actively being scanned out without worrying about the source > offset, since the source offset can change for a given framebuffer > (for example, when doing panning, or if an overlay is used for a HW > cursor). Actually, this patch is exactly same with my first implementation, so I completely understand your motivation. Anyway, I was focus on extended displays with one buffer, so I wrote modified dma base address. Thanks and Regards, - Seung-Woo Kim > > Best Regards, > -Daniel > >> >> Regards, >> - Seung-Woo Kim >> >>> >>> if (win_data->scan_flags & DRM_MODE_FLAG_INTERLACE) >>> ctx->interlace = true; >>> >> >> -- >> Seung-Woo Kim >> Samsung Software R Center >> -- >> > -- Seung-Woo Kim Samsung Software R Center -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] drm/exynos/mixer: use MXR_GRP_SXY_SY
Hello Daniel, On 2014년 05월 07일 23:14, Daniel Kurtz wrote: On Wed, May 7, 2014 at 1:14 PM, Seung-Woo Kim sw0312@samsung.com wrote: Hi Daniel, On 2014년 05월 05일 00:26, Daniel Kurtz wrote: Mixer hardware supports offsetting dma from start of source buffer using the MXR_GRP_SXY register. Signed-off-by: Daniel Kurtz djku...@chromium.org --- drivers/gpu/drm/exynos/exynos_mixer.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 475eb49..40cf39b 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -529,13 +529,11 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win) dst_x_offset = win_data-crtc_x; dst_y_offset = win_data-crtc_y; + src_x_offset = win_data-fb_x; + src_y_offset = win_data-fb_y; /* converting dma address base and source offset */ - dma_addr = win_data-dma_addr - + (win_data-fb_x * win_data-bpp 3) - + (win_data-fb_y * win_data-fb_width * win_data-bpp 3); - src_x_offset = 0; - src_y_offset = 0; + dma_addr = win_data-dma_addr; Basically, you are right and source offset register can be used. But because of limitation of resolution for mixer up to 1920x1080, I considered modified soruce dma address to set one frame buffer, which is bigger than 1920x1080, on to both fimd and hdmi. Hi Seung-Woo, I do not see why the maximum MIXER resolution matters for choosing between offsetting BASE or using SXY. Let's say you have one big 1920x1908 framebuffer, with a span of 1920, starting at dma_addr (there is no extra padding at the end of the line). Let's say you wanted the mixer to scan out 1920x1080 pixels starting from (0, 800) in the framebuffer, and start drawing them at (0,0) on the screen. What we currently do is: BASE = dma_addr + (800 * 1080 * 4) SPAN = 1920 SXY = SX(0) | SY(0) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) I am proposing we do: BASE = dma_addr SPAN = 1920 SXY = SX(0) | SY(800) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) In both cases, the mixer resolution is 1920x1080. In my test to show each half of big one framebuffer (3840 x 1080) to FIMD from 0 to 1079 and MIXER from 1080 to 3839 with exynos4210 and exynos4412, it was failed to show proper hdmi display. Also it is same for framebuffer (1920 x 2160). AFAIK, it is mainly because mixer dma has limitation of dma memory size. In this case, I set register as like: BASE = dma_addr /* 3840 x 1080 x 4 */ SPAN = 3840 SXY = SX(1920) | SY(0) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) or: BASE = dma_addr /* 1920 x 2160 x 4 */ SPAN = 1920 SXY = SX(0) | SY(1080) WH = W(1920) | H(1080) DXY = DX(0) | DY(0) but these two setting did not show hdmi display as I expected. So I used modified dma address. My motivation for wanting to program an un-modified dma_addr into BASE is so we can then just check BASE_S to determine from which buffer the mixer is actively being scanned out without worrying about the source offset, since the source offset can change for a given framebuffer (for example, when doing panning, or if an overlay is used for a HW cursor). Actually, this patch is exactly same with my first implementation, so I completely understand your motivation. Anyway, I was focus on extended displays with one buffer, so I wrote modified dma base address. Thanks and Regards, - Seung-Woo Kim Best Regards, -Daniel Regards, - Seung-Woo Kim if (win_data-scan_flags DRM_MODE_FLAG_INTERLACE) ctx-interlace = true; -- Seung-Woo Kim Samsung Software RD Center -- -- Seung-Woo Kim Samsung Software RD Center -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] drm/exynos/mixer: use MXR_GRP_SXY_SY
Hi Daniel, On 2014년 05월 05일 00:26, Daniel Kurtz wrote: > Mixer hardware supports offsetting dma from start of source buffer using > the MXR_GRP_SXY register. > > Signed-off-by: Daniel Kurtz > --- > drivers/gpu/drm/exynos/exynos_mixer.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c > b/drivers/gpu/drm/exynos/exynos_mixer.c > index 475eb49..40cf39b 100644 > --- a/drivers/gpu/drm/exynos/exynos_mixer.c > +++ b/drivers/gpu/drm/exynos/exynos_mixer.c > @@ -529,13 +529,11 @@ static void mixer_graph_buffer(struct mixer_context > *ctx, int win) > > dst_x_offset = win_data->crtc_x; > dst_y_offset = win_data->crtc_y; > + src_x_offset = win_data->fb_x; > + src_y_offset = win_data->fb_y; > > /* converting dma address base and source offset */ > - dma_addr = win_data->dma_addr > - + (win_data->fb_x * win_data->bpp >> 3) > - + (win_data->fb_y * win_data->fb_width * win_data->bpp >> 3); > - src_x_offset = 0; > - src_y_offset = 0; > + dma_addr = win_data->dma_addr; Basically, you are right and source offset register can be used. But because of limitation of resolution for mixer up to 1920x1080, I considered modified soruce dma address to set one frame buffer, which is bigger than 1920x1080, on to both fimd and hdmi. Regards, - Seung-Woo Kim > > if (win_data->scan_flags & DRM_MODE_FLAG_INTERLACE) > ctx->interlace = true; > -- Seung-Woo Kim Samsung Software R Center -- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/4] drm/exynos/mixer: use MXR_GRP_SXY_SY
Hi Daniel, On 2014년 05월 05일 00:26, Daniel Kurtz wrote: Mixer hardware supports offsetting dma from start of source buffer using the MXR_GRP_SXY register. Signed-off-by: Daniel Kurtz djku...@chromium.org --- drivers/gpu/drm/exynos/exynos_mixer.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c index 475eb49..40cf39b 100644 --- a/drivers/gpu/drm/exynos/exynos_mixer.c +++ b/drivers/gpu/drm/exynos/exynos_mixer.c @@ -529,13 +529,11 @@ static void mixer_graph_buffer(struct mixer_context *ctx, int win) dst_x_offset = win_data-crtc_x; dst_y_offset = win_data-crtc_y; + src_x_offset = win_data-fb_x; + src_y_offset = win_data-fb_y; /* converting dma address base and source offset */ - dma_addr = win_data-dma_addr - + (win_data-fb_x * win_data-bpp 3) - + (win_data-fb_y * win_data-fb_width * win_data-bpp 3); - src_x_offset = 0; - src_y_offset = 0; + dma_addr = win_data-dma_addr; Basically, you are right and source offset register can be used. But because of limitation of resolution for mixer up to 1920x1080, I considered modified soruce dma address to set one frame buffer, which is bigger than 1920x1080, on to both fimd and hdmi. Regards, - Seung-Woo Kim if (win_data-scan_flags DRM_MODE_FLAG_INTERLACE) ctx-interlace = true; -- Seung-Woo Kim Samsung Software RD Center -- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/8] extcon: arizona: Use devm_extcon_dev_register()
Hi,
On 2014년 04월 16일 19:27, Sangjung Woo wrote:
> Use the resource-managed extcon device register function (i.e.
> devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
> is attached with this function, that extcon device is automatically
> unregistered
> on driver detach. That reduces tiresome managing code.
>
> Signed-off-by: Sangjung Woo
> ---
> drivers/extcon/extcon-arizona.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
> index 98a14f6..40e6c0b 100644
> --- a/drivers/extcon/extcon-arizona.c
> +++ b/drivers/extcon/extcon-arizona.c
> @@ -1105,15 +1105,13 @@ static int arizona_extcon_probe(struct
> platform_device *pdev)
> info = devm_kzalloc(>dev, sizeof(*info), GFP_KERNEL);
> if (!info) {
> dev_err(>dev, "Failed to allocate memory\n");
> - ret = -ENOMEM;
> - goto err;
> + return -ENOMEM;
> }
>
> info->micvdd = devm_regulator_get(arizona->dev, "MICVDD");
> if (IS_ERR(info->micvdd)) {
> - ret = PTR_ERR(info->micvdd);
> dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
Assignment to ret is removed but it is still used here.
> - goto err;
> + return PTR_ERR(info->micvdd);
> }
>
> mutex_init(>lock);
> @@ -1155,11 +1153,11 @@ static int arizona_extcon_probe(struct
> platform_device *pdev)
> info->edev.dev.parent = arizona->dev;
> info->edev.supported_cable = arizona_cable;
>
> - ret = extcon_dev_register(>edev);
> + ret = devm_extcon_dev_register(>dev, >edev);
> if (ret < 0) {
> dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
> ret);
> - goto err;
> + return ret;
> }
>
> info->input = devm_input_allocate_device(>dev);
> @@ -1410,8 +1408,6 @@ err_rise:
> err_input:
> err_register:
> pm_runtime_disable(>dev);
> - extcon_dev_unregister(>edev);
> -err:
> return ret;
> }
>
> @@ -1445,7 +1441,6 @@ static int arizona_extcon_remove(struct platform_device
> *pdev)
> regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
> ARIZONA_JD1_ENA, 0);
> arizona_clk32k_disable(arizona);
> - extcon_dev_unregister(>edev);
>
> return 0;
> }
>
--
Seung-Woo Kim
Samsung Software R Center
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 8/8] extcon: arizona: Use devm_extcon_dev_register()
Hi,
On 2014년 04월 16일 19:27, Sangjung Woo wrote:
Use the resource-managed extcon device register function (i.e.
devm_extcon_dev_register()) instead of extcon_dev_register(). If extcon device
is attached with this function, that extcon device is automatically
unregistered
on driver detach. That reduces tiresome managing code.
Signed-off-by: Sangjung Woo sangjung@samsung.com
---
drivers/extcon/extcon-arizona.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/extcon/extcon-arizona.c b/drivers/extcon/extcon-arizona.c
index 98a14f6..40e6c0b 100644
--- a/drivers/extcon/extcon-arizona.c
+++ b/drivers/extcon/extcon-arizona.c
@@ -1105,15 +1105,13 @@ static int arizona_extcon_probe(struct
platform_device *pdev)
info = devm_kzalloc(pdev-dev, sizeof(*info), GFP_KERNEL);
if (!info) {
dev_err(pdev-dev, Failed to allocate memory\n);
- ret = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
info-micvdd = devm_regulator_get(arizona-dev, MICVDD);
if (IS_ERR(info-micvdd)) {
- ret = PTR_ERR(info-micvdd);
dev_err(arizona-dev, Failed to get MICVDD: %d\n, ret);
Assignment to ret is removed but it is still used here.
- goto err;
+ return PTR_ERR(info-micvdd);
}
mutex_init(info-lock);
@@ -1155,11 +1153,11 @@ static int arizona_extcon_probe(struct
platform_device *pdev)
info-edev.dev.parent = arizona-dev;
info-edev.supported_cable = arizona_cable;
- ret = extcon_dev_register(info-edev);
+ ret = devm_extcon_dev_register(pdev-dev, info-edev);
if (ret 0) {
dev_err(arizona-dev, extcon_dev_register() failed: %d\n,
ret);
- goto err;
+ return ret;
}
info-input = devm_input_allocate_device(pdev-dev);
@@ -1410,8 +1408,6 @@ err_rise:
err_input:
err_register:
pm_runtime_disable(pdev-dev);
- extcon_dev_unregister(info-edev);
-err:
return ret;
}
@@ -1445,7 +1441,6 @@ static int arizona_extcon_remove(struct platform_device
*pdev)
regmap_update_bits(arizona-regmap, ARIZONA_JACK_DETECT_ANALOGUE,
ARIZONA_JD1_ENA, 0);
arizona_clk32k_disable(arizona);
- extcon_dev_unregister(info-edev);
return 0;
}
--
Seung-Woo Kim
Samsung Software RD Center
--
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
