* [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check
@ 2017-05-20 9:48 Dan Carpenter
2017-05-22 3:20 ` Gang He
2017-05-23 2:20 ` Gang He
0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-05-20 9:48 UTC (permalink / raw)
To: ocfs2-devel
Hello Gang He,
The patch d56a8f32e4c6: "ocfs2: check/fix inode block for online file
check" from Mar 22, 2016, leads to the following static checker
warning:
fs/ocfs2/inode.c:179 ocfs2_iget()
warn: passing zero to 'ERR_PTR'
fs/ocfs2/inode.c
137 int sysfile_type)
138 {
139 int rc = 0;
^^^^^^
rc is zero.
140 struct inode *inode = NULL;
141 struct super_block *sb = osb->sb;
142 struct ocfs2_find_inode_args args;
143 journal_t *journal = OCFS2_SB(sb)->journal->j_journal;
144
145 trace_ocfs2_iget_begin((unsigned long long)blkno, flags,
146 sysfile_type);
147
148 /* Ok. By now we've either got the offsets passed to us by the
149 * caller, or we just pulled them off the bh. Lets do some
150 * sanity checks to make sure they're OK. */
151 if (blkno == 0) {
152 inode = ERR_PTR(-EINVAL);
153 mlog_errno(PTR_ERR(inode));
154 goto bail;
155 }
156
157 args.fi_blkno = blkno;
158 args.fi_flags = flags;
159 args.fi_ino = ino_from_blkno(sb, blkno);
160 args.fi_sysfile_type = sysfile_type;
161
162 inode = iget5_locked(sb, args.fi_ino, ocfs2_find_actor,
163 ocfs2_init_locked_inode, &args);
164 /* inode was *not* in the inode cache. 2.6.x requires
165 * us to do our own read_inode call and unlock it
166 * afterwards. */
167 if (inode == NULL) {
168 inode = ERR_PTR(-ENOMEM);
169 mlog_errno(PTR_ERR(inode));
170 goto bail;
171 }
172 trace_ocfs2_iget5_locked(inode->i_state);
173 if (inode->i_state & I_NEW) {
174 rc = ocfs2_read_locked_inode(inode, &args);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
or it's set here
175 unlock_new_inode(inode);
176 }
177 if (is_bad_inode(inode)) {
178 iput(inode);
179 inode = ERR_PTR(rc);
^^^^^^^^^^^^^^^^^^^
then we possibly set inode to NULL. The callers I looked at are not
expecting so it will result in a NULL deref.
180 goto bail;
181 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check
2017-05-20 9:48 [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check Dan Carpenter
@ 2017-05-22 3:20 ` Gang He
2017-05-23 2:20 ` Gang He
1 sibling, 0 replies; 4+ messages in thread
From: Gang He @ 2017-05-22 3:20 UTC (permalink / raw)
To: ocfs2-devel
Hello Dan,
Thank for your reporting.
I will send the patch for fixing this warning.
Thanks
Gang
>>>
> Hello Gang He,
>
> The patch d56a8f32e4c6: "ocfs2: check/fix inode block for online file
> check" from Mar 22, 2016, leads to the following static checker
> warning:
>
> fs/ocfs2/inode.c:179 ocfs2_iget()
> warn: passing zero to 'ERR_PTR'
>
> fs/ocfs2/inode.c
> 137 int sysfile_type)
> 138 {
> 139 int rc = 0;
> ^^^^^^
> rc is zero.
>
> 140 struct inode *inode = NULL;
> 141 struct super_block *sb = osb->sb;
> 142 struct ocfs2_find_inode_args args;
> 143 journal_t *journal = OCFS2_SB(sb)->journal->j_journal;
> 144
> 145 trace_ocfs2_iget_begin((unsigned long long)blkno, flags,
> 146 sysfile_type);
> 147
> 148 /* Ok. By now we've either got the offsets passed to us by
> the
> 149 * caller, or we just pulled them off the bh. Lets do some
> 150 * sanity checks to make sure they're OK. */
> 151 if (blkno == 0) {
> 152 inode = ERR_PTR(-EINVAL);
> 153 mlog_errno(PTR_ERR(inode));
> 154 goto bail;
> 155 }
> 156
> 157 args.fi_blkno = blkno;
> 158 args.fi_flags = flags;
> 159 args.fi_ino = ino_from_blkno(sb, blkno);
> 160 args.fi_sysfile_type = sysfile_type;
> 161
> 162 inode = iget5_locked(sb, args.fi_ino, ocfs2_find_actor,
> 163 ocfs2_init_locked_inode, &args);
> 164 /* inode was *not* in the inode cache. 2.6.x requires
> 165 * us to do our own read_inode call and unlock it
> 166 * afterwards. */
> 167 if (inode == NULL) {
> 168 inode = ERR_PTR(-ENOMEM);
> 169 mlog_errno(PTR_ERR(inode));
> 170 goto bail;
> 171 }
> 172 trace_ocfs2_iget5_locked(inode->i_state);
> 173 if (inode->i_state & I_NEW) {
> 174 rc = ocfs2_read_locked_inode(inode, &args);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> or it's set here
>
> 175 unlock_new_inode(inode);
> 176 }
> 177 if (is_bad_inode(inode)) {
> 178 iput(inode);
> 179 inode = ERR_PTR(rc);
> ^^^^^^^^^^^^^^^^^^^
> then we possibly set inode to NULL. The callers I looked at are not
> expecting so it will result in a NULL deref.
>
> 180 goto bail;
> 181 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check
2017-05-20 9:48 [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check Dan Carpenter
2017-05-22 3:20 ` Gang He
@ 2017-05-23 2:20 ` Gang He
2017-05-23 4:26 ` Dan Carpenter
1 sibling, 1 reply; 4+ messages in thread
From: Gang He @ 2017-05-23 2:20 UTC (permalink / raw)
To: ocfs2-devel
Hi Dan,
What static checker tool did you use? could you paste the detailed warning message to the list too?
Since I need to write this information into the patch comments due to Andrew Morton's suggestion.
Thanks a lot.
Gang
>>>
> Hello Gang He,
>
> The patch d56a8f32e4c6: "ocfs2: check/fix inode block for online file
> check" from Mar 22, 2016, leads to the following static checker
> warning:
>
> fs/ocfs2/inode.c:179 ocfs2_iget()
> warn: passing zero to 'ERR_PTR'
>
> fs/ocfs2/inode.c
> 137 int sysfile_type)
> 138 {
> 139 int rc = 0;
> ^^^^^^
> rc is zero.
>
> 140 struct inode *inode = NULL;
> 141 struct super_block *sb = osb->sb;
> 142 struct ocfs2_find_inode_args args;
> 143 journal_t *journal = OCFS2_SB(sb)->journal->j_journal;
> 144
> 145 trace_ocfs2_iget_begin((unsigned long long)blkno, flags,
> 146 sysfile_type);
> 147
> 148 /* Ok. By now we've either got the offsets passed to us by
> the
> 149 * caller, or we just pulled them off the bh. Lets do some
> 150 * sanity checks to make sure they're OK. */
> 151 if (blkno == 0) {
> 152 inode = ERR_PTR(-EINVAL);
> 153 mlog_errno(PTR_ERR(inode));
> 154 goto bail;
> 155 }
> 156
> 157 args.fi_blkno = blkno;
> 158 args.fi_flags = flags;
> 159 args.fi_ino = ino_from_blkno(sb, blkno);
> 160 args.fi_sysfile_type = sysfile_type;
> 161
> 162 inode = iget5_locked(sb, args.fi_ino, ocfs2_find_actor,
> 163 ocfs2_init_locked_inode, &args);
> 164 /* inode was *not* in the inode cache. 2.6.x requires
> 165 * us to do our own read_inode call and unlock it
> 166 * afterwards. */
> 167 if (inode == NULL) {
> 168 inode = ERR_PTR(-ENOMEM);
> 169 mlog_errno(PTR_ERR(inode));
> 170 goto bail;
> 171 }
> 172 trace_ocfs2_iget5_locked(inode->i_state);
> 173 if (inode->i_state & I_NEW) {
> 174 rc = ocfs2_read_locked_inode(inode, &args);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> or it's set here
>
> 175 unlock_new_inode(inode);
> 176 }
> 177 if (is_bad_inode(inode)) {
> 178 iput(inode);
> 179 inode = ERR_PTR(rc);
> ^^^^^^^^^^^^^^^^^^^
> then we possibly set inode to NULL. The callers I looked at are not
> expecting so it will result in a NULL deref.
>
> 180 goto bail;
> 181 }
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check
2017-05-23 2:20 ` Gang He
@ 2017-05-23 4:26 ` Dan Carpenter
0 siblings, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2017-05-23 4:26 UTC (permalink / raw)
To: ocfs2-devel
On Mon, May 22, 2017 at 08:20:02PM -0600, Gang He wrote:
> Hi Dan,
>
> What static checker tool did you use? could you paste the detailed warning message to the list too?
> Since I need to write this information into the patch comments due to Andrew Morton's suggestion.
>
It's some Smatch stuff that I was working on yesterday. I'm probably
going to publish this in a month or two. It's hard to say because, of
course, returning NULL is often valid.
Anyway, the warning is triggered when you pass zero to ERR_PTR().
fs/ocfs2/inode.c:179 ocfs2_iget()
warn: passing zero to 'ERR_PTR'
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-05-23 4:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-20 9:48 [Ocfs2-devel] [bug report] ocfs2: check/fix inode block for online file check Dan Carpenter
2017-05-22 3:20 ` Gang He
2017-05-23 2:20 ` Gang He
2017-05-23 4:26 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.